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.

Sign up for GitHub

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

Jump to bottom

Implement --emit-index-annotation to annotate source index for each package #2926

Merged
merged 12 commits into from
Apr 10, 2024
Merged

Implement --emit-index-annotation to annotate source index for each package #2926

merged 12 commits into from
Apr 10, 2024

Conversation

ChannyClaus
Copy link
Contributor

@ChannyClaus ChannyClaus commented Apr 9, 2024 •
edited
Loading

Summary

resolves #2852

Test Plan

add a couple of tests:

  • one covering the simplest case with all packages pulled from a single index.
  • another where packages are pull from two distinct indices.

tested manually as well:

$ (echo 'pandas'; echo 'torch') | UV_EXTRA_INDEX_URL='https://download.pytorch.org/whl/cpu' cargo run pip compile - --include-indices 
    Finished dev [unoptimized + debuginfo] target(s) in 0.60s
     Running `target/debug/uv pip compile - --include-indices`
Resolved 15 packages in 686ms
# This file was autogenerated by uv via the following command:
#    uv pip compile - --include-indices
filelock==3.9.0
    # via torch
    # from https://download.pytorch.org/whl/cpu
fsspec==2023.4.0
    # via torch
    # from https://download.pytorch.org/whl/cpu
jinja2==3.1.2
    # via torch
    # from https://download.pytorch.org/whl/cpu
markupsafe==2.1.3
    # via jinja2
    # from https://download.pytorch.org/whl/cpu
mpmath==1.3.0
    # via sympy
    # from https://download.pytorch.org/whl/cpu
networkx==3.2.1
    # via torch
    # from https://download.pytorch.org/whl/cpu
numpy==1.26.3
    # via pandas
    # from https://download.pytorch.org/whl/cpu
pandas==2.2.1
    # from https://pypi.org/simple
python-dateutil==2.9.0.post0
    # via pandas
    # from https://pypi.org/simple
pytz==2024.1
    # via pandas
    # from https://pypi.org/simple
six==1.16.0
    # via python-dateutil
    # from https://pypi.org/simple
sympy==1.12
    # via torch
    # from https://download.pytorch.org/whl/cpu
torch==2.2.2
    # from https://download.pytorch.org/whl/cpu
typing-extensions==4.8.0
    # via torch
    # from https://download.pytorch.org/whl/cpu
tzdata==2024.1
    # via pandas
    # from https://pypi.org/simple

@zanieb
Copy link
Member

zanieb commented Apr 9, 2024

Nice start! Should we make this opt-in?

@charliermarsh
Copy link
Member

Yeah this should be opt-in.

@charliermarsh charliermarsh self-assigned this Apr 10, 2024
@charliermarsh charliermarsh added the enhancement New feature or request label Apr 10, 2024
@ChannyClaus
Copy link
Contributor Author

any pointers on which index to use for the test case that uses multiple indices? --exclude-newer 2024-03-25T00:00:00Z with https://download.pytorch.org/whl/cpu would fail with the below 😢 .

warning: torch-0.1-cp35-cp35m-macosx_10_6_x86_64.whl is missing an upload date, but user provided: 2024-03-25 00:00:00 UTC

@ChannyClaus ChannyClaus marked this pull request as ready for review April 10, 2024 03:24
@charliermarsh
Copy link
Member

Can you use https://test.pypi.org/simple?

@ChannyClaus
Copy link
Contributor Author

bahaha thanks for not uploading uv to the test index 😆

ChannyClaus
ChannyClaus commented Apr 10, 2024
View reviewed changes
crates/uv-resolver/src/resolution.rs Outdated
@@ -714,6 +728,14 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> {
// Write the line as is.
writeln!(f, "{line}")?;
}

if self.include_indices && node.index().is_some() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should this conditional include self.include_annotations?

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 it's okay as-is.

@charliermarsh charliermarsh self-requested a review April 10, 2024 14:24
charliermarsh
charliermarsh reviewed Apr 10, 2024
View reviewed changes
crates/uv-resolver/src/resolution.rs Outdated
@@ -505,6 +506,8 @@ pub struct DisplayResolutionGraph<'a> {
/// Whether to include annotations in the output, to indicate which dependency or dependencies
/// requested each package.
include_annotations: bool,
/// Whether to include indices in the output, to indicate which index was used for each package.
include_indices: bool,
Copy link
Member

Choose a reason for hiding this comment

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

I know it's tedious but I'm going to change this to indexes to match the terminology that pip uses in its documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/shrug i can run through these real quick

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@charliermarsh charliermarsh changed the title add index in pip compile output Implement --emit-index-annotation to annotate source index for each package Apr 10, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) April 10, 2024 15:58
@charliermarsh charliermarsh merged commit 7cd98d2 into astral-sh:main Apr 10, 2024
36 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Apr 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

@charliermarsh charliermarsh

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Annotate the index from which a package is pulled in uv pip compile
3 participants
@ChannyClaus @zanieb @charliermarsh