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

Implement "Requires" field in pip show #2347

Merged
merged 10 commits into from
Mar 12, 2024
Merged

Conversation

ChannyClaus
Copy link
Contributor

Summary

Follow-up for 395be44

adds Requires field to pip show output.

I've aimed to make it behave exactly the same as pip does for now, but there seem to be subtle issues that may require some discussion going forward:

  • Should uv pip show support extras? pip has an open issue for it, but currently does not support Add support for outputting a list of extras and their requirements. pypa/pip#4824.
  • Relatedly, Requred-by field (not implemented in this PR) in pip show currently doesn't take the extras into account transparently, i.e. when PySocks has been installed as an extra for requests[socks], pip show PySocks doesn't have requests or requests[socks] under Requred-by field. Should uv pip show for now just replicate pip's behavior for now for simplicity and parity or try to cover the extras for completeness?

Test Plan

Added a couple of tests:

  1. requests==2.31.0 has four dependencies that would be ordered differently unless sorted. Additionally, it has two dependencies that are optionally included for extras.
  2. pandas==2.1.3 depends on different versions of numpy depending on the python version used.

@ChannyClaus ChannyClaus marked this pull request as ready for review March 10, 2024 23:45
}

#[test]
#[cfg(not(windows))]
Copy link
Contributor Author

@ChannyClaus ChannyClaus Mar 10, 2024

Choose a reason for hiding this comment

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

not super clear why windows doesn't pull tzdata https://github.com/astral-sh/uv/actions/runs/8225296455/job/22490152644; seems like it's not a platform-dependent dependency https://github.com/pandas-dev/pandas/blob/871f01b5582fc737a63f17c1d9027eb6a2099912/pyproject.toml#L37. may continue to look while having this open for review 😭 (probably not a blocking issue?)

Copy link
Member

Choose a reason for hiding this comment

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

We have a windows filter hack in the snapshots for our common test packages:

// The optional leading +/- is for install logs, the optional next line is for lock files
let windows_only_deps = [
("( [+-] )?colorama==\\d+(\\.[\\d+])+\n( # via .*\n)?"),
("( [+-] )?colorama==\\d+(\\.[\\d+])+\\s+(# via .*\n)?"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+\n( # via .*\n)?"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+\\s+(# via .*\n)?"),
];

Could you use a pure python package for the test case? Pandas and numpy wheel are big due to their native modules (something like 5-20 MB) and since we run the tests with no cache these add up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to use click.

$ du -sh /Users/chan.kang/test/uv/venv/lib/python3.12/site-packages/click
824K	/Users/chan.kang/test/uv/venv/lib/python3.12/site-packages/click

@konstin
Copy link
Member

konstin commented Mar 11, 2024

Should uv pip show support extras? pip has an open issue for it, but currently does not support pypa/pip#4824.

That would be a nice addition, feel free to make a follow-up PR.

Relatedly, Requred-by field (not implemented in this PR) in pip show currently doesn't take the extras into account transparently, i.e. when PySocks has been installed as an extra for requests[socks], pip show PySocks doesn't have requests or requests[socks] under Requred-by field. Should uv pip show for now just replicate pip's behavior for now for simplicity and parity or try to cover the extras for completeness?

There's unfortunately no tracking for which extras get installed, so we don't know if an extra was activated or not.

.into_iter()
.filter(|req| req.evaluate_markers(markers, &[]))
.map(|req| req.name.to_string())
.collect::<Vec<_>>();
Copy link
Member

Choose a reason for hiding this comment

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

Can you try collecting into a BTreeSet instead? There are some cases where names occur twice in the requirements. This would remove those duplicates and skip the sorting step

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you happen to remember which particular package has duplicates (was thinking if I should add a test using it)? https://github.com/pypa/pip/blob/fb5f63f0f4c3a204ada8353ce9858d4d4b777c2a/src/pip/_internal/commands/show.py#L104 does seem to suggest the same.

crates/uv/tests/pip_show.rs Outdated Show resolved Hide resolved
@ChannyClaus ChannyClaus requested a review from konstin March 12, 2024 02:40
@charliermarsh charliermarsh added enhancement New feature or improvement to existing functionality cli Related to the command line interface labels Mar 12, 2024
@charliermarsh charliermarsh changed the title implement "Requires" field in pip show Implement "Requires" field in pip show Mar 12, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) March 12, 2024 04:23
@charliermarsh charliermarsh force-pushed the main branch 2 times, most recently from f080835 to 23bc073 Compare March 12, 2024 04:29
@charliermarsh charliermarsh merged commit 9bb548d into astral-sh:main Mar 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command line interface enhancement New feature or improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants