Skip to content

Editable project location and Required-by for pip show#2589

Merged
zanieb merged 5 commits intoastral-sh:mainfrom
eth3lbert:pip-show
Mar 25, 2024
Merged

Editable project location and Required-by for pip show#2589
zanieb merged 5 commits intoastral-sh:mainfrom
eth3lbert:pip-show

Conversation

@eth3lbert
Copy link
Copy Markdown
Contributor

@eth3lbert eth3lbert commented Mar 21, 2024 •
edited by zanieb
Loading

Summary

  • Displays missing packages as single-line warnings.
  • Adds support for Editable project location and Required-by fields in pip show.

Part of #2526.

@eth3lbert eth3lbert changed the title editable project location and required-by for pip show Editable project location and Required-by for pip show Mar 21, 2024
zanieb
zanieb reviewed Mar 21, 2024
View reviewed changes
Comment thread crates/uv/src/commands/pip_show.rs Outdated
zanieb
zanieb reviewed Mar 21, 2024
View reviewed changes
Comment thread crates/uv/src/commands/pip_show.rs
return Ok(ExitStatus::Failure);
}

// Since Requires and Required-by fields need data parsed from metadata, especially the
Copy link
Copy Markdown
Member

@zanieb zanieb Mar 21, 2024

Choose a reason for hiding this comment

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

@charliermarsh or @konstin could you review this section?

Copy link
Copy Markdown
Member

@konstin konstin Mar 26, 2024

Choose a reason for hiding this comment

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

Sorry i missed the ping, code looks good

zanieb
zanieb reviewed Mar 21, 2024
View reviewed changes
Comment thread crates/uv/tests/pip_show.rs
Location: [WORKSPACE_DIR]/site-packages
Editable project location: [EDITABLE_INSTALLS_PREFIX]poetry_editable
Requires: anyio
Required-by:
Copy link
Copy Markdown
Member

@zanieb zanieb Mar 21, 2024

Choose a reason for hiding this comment

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

I wonder if we should omit these when they're empty

Copy link
Copy Markdown
Contributor Author

@eth3lbert eth3lbert Mar 21, 2024

Choose a reason for hiding this comment

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

I followed the approach used by pip. I have no preference, either option is fine.

Copy link
Copy Markdown
Contributor

@danielhollas danielhollas Mar 21, 2024

Choose a reason for hiding this comment

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

I'd vote to keep pip bwhaviour, otherwise users can wonder if it is even implemented.

Copy link
Copy Markdown
Member

@zanieb zanieb Mar 21, 2024

Choose a reason for hiding this comment

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

I don't think being worried that people will wonder if it it's implemented makes sense as a long-term choice. It seems likely if you care about this feature that you'll have at least one package with Required-by set. I don't have strong feelings but I'd lean towards omitting empty fields. Curious for others' thoughts though cc @konstin

Copy link
Copy Markdown
Member

@konstin konstin Mar 26, 2024

Choose a reason for hiding this comment

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

I don't have an opinion on that, we could add (none) as placeholder if it helps

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Mar 21, 2024

Thanks for contributing!

@danielhollas
Copy link
Copy Markdown
Contributor

danielhollas commented Mar 21, 2024

Awesome.

One small thing from the linked issue: if it isn't too much extra work it would be nice to display Summary and Home page as well. Those are nice for packages with which one is not familiar.

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Mar 21, 2024

@danielhollas I wouldn't expand the scope of this pull request, it's better to implement things separately.

@danielhollas
Copy link
Copy Markdown
Contributor

danielhollas commented Mar 21, 2024

Totally agree! But then please don't close the issue since it's not fully resolved : 😅

@eth3lbert
Copy link
Copy Markdown
Contributor Author

eth3lbert commented Mar 21, 2024 •
edited
Loading

The implementation itself shouldn't be difficult. However, those fields also come from metadata, which are not stored in Metadata23 (perhaps to keep the footprint size small?). In this case, we need to decide how to proceed: should we simply include more fields here, or introduce a separate struct that provides the full metadata?

zanieb
zanieb approved these changes Mar 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@zanieb zanieb left a comment

Choose a reason for hiding this comment

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

Thanks!

@zanieb zanieb merged commit 6d7b2c7 into astral-sh:main Mar 25, 2024
@zanieb zanieb added cli Related to the command line interface enhancement New feature or improvement to existing functionality labels Mar 25, 2024
@eth3lbert eth3lbert deleted the pip-show branch March 25, 2024 19:46
@charliermarsh charliermarsh mentioned this pull request Mar 25, 2024
Merged
charliermarsh added a commit that referenced this pull request Mar 25, 2024
@charliermarsh
Add rustc-hash to uv crate (#2660)
f375334 
## Summary

This is required as of #2589, but
isn't a direct dependency. I'm not sure how that PR passed CI exactly.
zanieb pushed a commit that referenced this pull request Mar 25, 2024
@eth3lbert @zanieb
Editable project location and Required-by for pip show (#2589)
17a6789 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

- Displays missing packages as single-line warnings.
- Adds support for `Editable project location` and `Required-by` fields
in `pip show`.

Part of #2526.
zanieb pushed a commit that referenced this pull request Mar 25, 2024
@eth3lbert @zanieb
Editable project location and Required-by for pip show (#2589)
cf2210b 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

- Displays missing packages as single-line warnings.
- Adds support for `Editable project location` and `Required-by` fields
in `pip show`.

Part of #2526.
zanieb added a commit that referenced this pull request Mar 26, 2024
@zanieb
Editable project location and Required-by for pip show (#2589)
e680966 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

- Displays missing packages as single-line warnings.
- Adds support for `Editable project location` and `Required-by` fields
in `pip show`.

Part of #2526.

# Conflicts:
#	crates/uv-dispatch/src/lib.rs
#	crates/uv-resolver/src/resolver/mod.rs
#	crates/uv-resolver/tests/resolver.rs
#	crates/uv-traits/src/lib.rs
#	crates/uv/src/commands/pip_compile.rs
zanieb added a commit that referenced this pull request Mar 27, 2024
@zanieb
Editable project location and Required-by for pip show (#2589)
c3ae948 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

- Displays missing packages as single-line warnings.
- Adds support for `Editable project location` and `Required-by` fields
in `pip show`.

Part of #2526.

# Conflicts:
#	crates/uv-dispatch/src/lib.rs
#	crates/uv-resolver/src/resolver/mod.rs
#	crates/uv-resolver/tests/resolver.rs
#	crates/uv-traits/src/lib.rs
#	crates/uv/src/commands/pip_compile.rs

# Conflicts:
#	crates/uv/src/commands/pip_sync.rs
zanieb added a commit that referenced this pull request Mar 27, 2024
@zanieb
Editable project location and Required-by for pip show (#2589)
a0adc73 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

- Displays missing packages as single-line warnings.
- Adds support for `Editable project location` and `Required-by` fields
in `pip show`.

Part of #2526.

# Conflicts:
#	crates/uv-dispatch/src/lib.rs
#	crates/uv-resolver/src/resolver/mod.rs
#	crates/uv-resolver/tests/resolver.rs
#	crates/uv-traits/src/lib.rs
#	crates/uv/src/commands/pip_compile.rs

# Conflicts:
#	crates/uv/src/commands/pip_sync.rs
zanieb added a commit that referenced this pull request Mar 27, 2024
@zanieb
Editable project location and Required-by for pip show (#2589)
44f52b5 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

- Displays missing packages as single-line warnings.
- Adds support for `Editable project location` and `Required-by` fields
in `pip show`.

Part of #2526.

# Conflicts:
#	crates/uv-dispatch/src/lib.rs
#	crates/uv-resolver/src/resolver/mod.rs
#	crates/uv-resolver/tests/resolver.rs
#	crates/uv-traits/src/lib.rs
#	crates/uv/src/commands/pip_compile.rs

# Conflicts:
#	crates/uv/src/commands/pip_sync.rs
zanieb added a commit that referenced this pull request Mar 27, 2024
@zanieb
Editable project location and Required-by for pip show (#2589)
5afcfda 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

- Displays missing packages as single-line warnings.
- Adds support for `Editable project location` and `Required-by` fields
in `pip show`.

Part of #2526.

# Conflicts:
#	crates/uv-dispatch/src/lib.rs
#	crates/uv-resolver/src/resolver/mod.rs
#	crates/uv-resolver/tests/resolver.rs
#	crates/uv-traits/src/lib.rs
#	crates/uv/src/commands/pip_compile.rs

# Conflicts:
#	crates/uv/src/commands/pip_sync.rs
@BrewTestBot BrewTestBot mentioned this pull request Mar 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@konstin konstin konstin left review comments

@zanieb zanieb zanieb approved these changes

+1 more reviewer

@danielhollas danielhollas danielhollas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cli Related to the command line interface enhancement New feature or improvement to existing functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@eth3lbert @zanieb @danielhollas @konstin