Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show: show editable location instead if package is editable (#11638) #11639

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

doronz88
Copy link
Contributor

@doronz88 doronz88 commented Dec 6, 2022

No description provided.

@doronz88 doronz88 force-pushed the bugfix/pip_show_location branch 4 times, most recently from 0e30b3a to 63edd62 Compare December 6, 2022 12:45
@doronz88
Copy link
Contributor Author

doronz88 commented Dec 6, 2022

@uranusjr can you look at the CI failure? I can't tell why the error is caused

@uranusjr
Copy link
Member

uranusjr commented Dec 7, 2022

Something’s wrong in CI, not your fault.

@@ -119,7 +119,7 @@ def _get_requiring_packages(current_dist: BaseDistribution) -> Iterator[str]:
yield _PackageInfo(
name=dist.raw_name,
version=str(dist.version),
location=dist.location or "",
location=dist.editable_project_location or dist.location or "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a new field, since location does have a valid meaning for editables too and to avoid mixing different semantics in the same field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the location shown in pip list and that's the actual location from the user's point of view.
what do you suggest as the other field? i think showing an additional editable location will just make it look confusing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the location shown in pip list and that's the actual location from the user's point of view.

In pip list, the column title is Editable project location and is populated for editables only.
pip list -v shows an additional Location column with dist.location which is where the project metadata is (which is different from the project location for PEP 660 editables).
It is similar in pip inspect and pip list --format=json.

So in pip show, it makes sense to show a new Editable project location field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@doronz88 doronz88 force-pushed the bugfix/pip_show_location branch 2 times, most recently from 4231cc1 to 773a8f5 Compare December 11, 2022 07:09
Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! There are a few tests that need updating to cope with the new field.

src/pip/_internal/commands/show.py Outdated Show resolved Hide resolved
@doronz88 doronz88 force-pushed the bugfix/pip_show_location branch 5 times, most recently from cea09c1 to 96732d8 Compare December 12, 2022 07:13
@doronz88
Copy link
Contributor Author

@uranusjr can you verify this PR? it passes CI now

@sbidoul sbidoul merged commit 32634e5 into pypa:main Dec 19, 2022
@sbidoul
Copy link
Member

sbidoul commented Dec 19, 2022

Thanks for the contrib @doronz88 !

@doronz88 doronz88 deleted the bugfix/pip_show_location branch December 19, 2022 08:53
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants