-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add is_yanked to installation report #12224
Conversation
19df53f
to
56babba
Compare
a3b5bd5
to
4652c95
Compare
4652c95
to
361b02b
Compare
Co-authored-by: Paul Moore <p.f.moore@gmail.com>
Hi @pfmoore 👋 anything else from your side? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contrib! Code looks good to me. Could you update the documentation page too?
@@ -23,6 +23,9 @@ def _install_req_to_dict(cls, ireq: InstallRequirement) -> Dict[str, Any]: | |||
# includes editable requirements), and false if the requirement was | |||
# downloaded from a PEP 503 index or --find-links. | |||
"is_direct": ireq.is_direct, | |||
# is_yanked is true if the requirement was yanked from the index, but | |||
# was still selected by pip to conform to PEP 592. | |||
"is_yanked": ireq.link.is_yanked if ireq.link else False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know in which circumstances ireq.link
can be None ?
Perhaps we should output "is_yanked": None
in that case, to express that we don't know if it's yanked or not, rather the False which asserts it is not yanked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when ireq.link
is None, it's not coming from a pypi index. since the notion of yanking only applies to pypi indices, I think returning False here is safe. but please correct me if I'm wrong @pfmoore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when ireq.link is None, it's not coming from a pypi index
If that's the reason of ireq.link is None, then it's fine to return is_yanked=False in such case, indeed.
Hi @sbidoul 👋 it looks like those docs are a bit outdated (e.g. |
Sorry I had linked the inspection report instead of the installation report (link now fixed). AFAIK the installation report reference page is up-to-date. Even if it was not, that should not be a reason to let it drift even more :) |
@sbidoul done 🤙 |
@@ -56,6 +56,9 @@ package with the following properties: | |||
URL reference. `false` if the requirements was provided as a name and version | |||
specifier. | |||
|
|||
- `is_yanked`: `true` if the requirement was yanked from the index, but was still | |||
selected by pip conform to [PEP 592](https://peps.python.org/pep-0592/#installers). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selected by pip conform to [PEP 592](https://peps.python.org/pep-0592/#installers). | |
selected by pip in comformance with [PEP 592](https://peps.python.org/pep-0592/#installers). |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im no native, but looks like 'conform to' and 'conform with' are both fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all four seem to occur, conform to
seems most common on ngrams
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not native either :)
This PR adds
is_yanked
to the installation report as a machine-readable alternative to #12225