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 new command 'pip index versions' #8978

Merged
merged 6 commits into from
Jun 11, 2021

Conversation

NoahGorny
Copy link
Contributor

Instead of doing dirty tricks in order to get all available versions of a package(https://stackoverflow.com/questions/4888027/python-and-pip-list-all-versions-of-a-package-thats-available), we can now call
pip search <package> --list-all-versions in order to see all of the available versions!

Lemme know what you think @uranusjr @pradyunsg!

@NoahGorny
Copy link
Contributor Author

Not sure what issue number to use with the NEWS entry... I remember that using the PR number is not a good idea :(

@McSinyx
Copy link
Contributor

McSinyx commented Oct 10, 2020

Not sure what issue number to use with the NEWS entry...

It could be GH-7975 I think. I'm glad that this i picked up after I totally forgot about it 😅

@NoahGorny
Copy link
Contributor Author

Not sure what issue number to use with the NEWS entry...

It could be GH-7975 I think. I'm glad that this i picked up after I totally forgot about it sweat_smile

I see, added a news entry now 😄

@NoahGorny
Copy link
Contributor Author

@pradyunsg @uranusjr @McSinyx Im humbly bumping this PR, because I want to know what do you think about this feature offer. should this be discussed in the python forums?

@pfmoore
Copy link
Member

pfmoore commented Oct 14, 2020

Looks OK to me. I'm a little bit ambivalent about adding this to search, but simply because the existing search is so bad that I'm not entirely sure we want to have people thinking it's useful for anything 😉 But I've no objection to this going in.

@pradyunsg
Copy link
Member

pradyunsg commented Oct 14, 2020

I'd prefer to hold off, since we have the UX study doing a full pip functionality overview right now, and I don't want us to make changes to the commands in the middle of that.

/cc @ei8fdb @nlhkabu in case they want to weigh in or something.

@ei8fdb
Copy link
Contributor

ei8fdb commented Oct 14, 2020

Hi @NoahGorny! As @pradyunsg mentions, we are doing some UX research on what users need from pip search. We're still figuring out the results, but this PR will allow us to test something with users. Thanks for this.

I'm going to try out your PR to see the output. Ideally we'd be able to usability test this with users and get feedback. Would you be OK with discussing usability improvements we might recommend?

I've tried it out and so far so good. 😄

@NoahGorny
Copy link
Contributor Author

Hi @NoahGorny! As @pradyunsg mentions, we are doing some UX research on what users need from pip search. We're still figuring out the results, but this PR will allow us to test something with users. Thanks for this.

I'm going to try out your PR to see the output. Ideally we'd be able to usability test this with users and get feedback. Would you be OK with discussing any usability changes?

I welcome any feedback and suggestions, and happy to help! I think you guys are doing an amazing job improving pip which is one of the most important and used tools today!

I will try to be as available as I can, and I am open to new ideas and changes 😄

@NoahGorny
Copy link
Contributor Author

@ei8fdb @pradyunsg are there any updates on things I should do? I do not want this PR to be forgotten 😢

@uranusjr
Copy link
Member

uranusjr commented Dec 3, 2020

I like overloading pip search since the command is a mess already, and we likely want to come up with something entirely different (e.g. pip find) to replace it. We can also build on this and add more experiements before committing them into the new command.

@uranusjr
Copy link
Member

uranusjr commented Dec 3, 2020

I wonder if it would be better to use --use-feature for this though. The flag would somehow communicate the feature is more provisional and subject to change in the future.

@NoahGorny
Copy link
Contributor Author

I wonder if it would be better to use --use-feature for this though. The flag would somehow communicate the feature is more provisional and subject to change in the future.

We can just add a warning that this API is experimental, and if we want to move away from it, we can deprecate it in the future

@pradyunsg
Copy link
Member

We can just add a warning that this API is experimental, and if we want to move away from it, we can deprecate it in the future

I'm on board if we're letting ourselves remove this in a future release without a deprecation cycle.

WARNING: pip search --list-all-versions is currently an experimental flag. It may be removed in a future release without prior warning.

@NoahGorny
Copy link
Contributor Author

We can just add a warning that this API is experimental, and if we want to move away from it, we can deprecate it in the future

I'm on board if we're letting ourselves remove this in a future release without a deprecation cycle.

WARNING: pip search --list-all-versions is currently an experimental flag. It may be removed in a future release without prior warning.

Why not use a deprecation cycle? it only requires us to remember deprecating and removing after a while

@NoahGorny
Copy link
Contributor Author

I switched to json API as the xml-rpc is deprecated and causing me trouble. Not sure this is the right way to do it as I handled the http errors myself

@uranusjr uranusjr added this to the 21.0 milestone Dec 8, 2020
@uranusjr
Copy link
Member

uranusjr commented Dec 8, 2020

I’m adding this to the 21.0 milestone because

  1. People need an alternative to the legacy resolver’s pip install foo== hack after the legacy resolver is removed entirely.
  2. We should provide an alternative, and this is a choice. Even if we decide to not go with this, we must implement something else, and having this PR in the milestone makes sure we do that.

@NoahGorny NoahGorny force-pushed the pip-get-all-versions-of-package branch from 3d764df to 3699993 Compare December 11, 2020 12:18
@pfmoore
Copy link
Member

pfmoore commented Jan 9, 2021

A quick note. Pip 21.0 is due for release in about 2 weeks (see #9282). This item is on the 21.0 milestone, so it either needs to be implemented and merged to master before then, or it will miss the release (and I'll move it to the 21.1 milestone).

@NoahGorny
Copy link
Contributor Author

I mostly wait for people to have feedback on this, as I use the JSON API and want to hear ppl opinion about it

@pradyunsg
Copy link
Member

People need an alternative to the legacy resolver’s pip install foo== hack after the legacy resolver is removed entirely.

We've essentially restored this with #9228 being closed. I don't think this blocks 21.0, but, we should figure out what we really want to do here.

@uranusjr
Copy link
Member

It'd be useful of we could get an update to the UX research about this.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 4, 2021
@uranusjr uranusjr removed this from the 21.0 milestone Mar 7, 2021
@earonesty
Copy link

is the only thing holding this back a need to rebase?

@jgspratt
Copy link

Who can merge this?

@ei8fdb ei8fdb closed this May 18, 2021
@ei8fdb ei8fdb reopened this May 18, 2021
@uranusjr
Copy link
Member

Looks reasonable to me. So, as the first shot at this, @NoahGorny could you modify this PR to implement pip index versions <package> instead? The rest can come later after we get this in.

@NoahGorny
Copy link
Contributor Author

Looks reasonable to me. So, as the first shot at this, @NoahGorny could you modify this PR to implement pip index versions <package> instead? The rest can come later after we get this in.

I see, will do in the following days!

I think you can try and review my changes in the meantime.. I am not super happy about the way I retrieve the information. I should probably split the JSON API logic to a separate class, or use the simple API instead.

@uranusjr
Copy link
Member

Let's use the simple API instead. You should be able to reuse may logic in pip._internal.index

@NoahGorny NoahGorny force-pushed the pip-get-all-versions-of-package branch from 77a3e70 to 2e062e0 Compare June 10, 2021 23:27
@NoahGorny
Copy link
Contributor Author

Hi @uranusjr @pradyunsg, I have updated the PR as needed, added tests, and I feel like its ready for a review.

Let me know what you think 😄

@NoahGorny NoahGorny force-pushed the pip-get-all-versions-of-package branch from 2e062e0 to dce8972 Compare June 11, 2021 01:26
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Happy to see this merged, and we can iterate on this.

A few non-blocking suggested rephrasings.

news/7975.feature.rst Outdated Show resolved Hide resolved
src/pip/_internal/commands/__init__.py Outdated Show resolved Hide resolved
src/pip/_internal/commands/index.py Outdated Show resolved Hide resolved
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

A general note: The current implementation lists only applicable versions of a project, but that behaviour (same as pip install etc.) sometimes leads to a common user confusion: Why does pip not see a newly published version?

I guess the current impleemntation is probably good enough as an initial effort, but eventually we probably need to also list “found but not applicable” versions as well (and potentially also why they are not applicable). I’m not sure how it can be done best, however.

news/7975.feature.rst Outdated Show resolved Hide resolved
src/pip/_internal/commands/index.py Outdated Show resolved Hide resolved
pradyunsg
pradyunsg previously approved these changes Jun 11, 2021
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

I clicked the wrong button. :)

@pradyunsg pradyunsg dismissed their stale review June 11, 2021 09:17

There are other concerns. :)

@NoahGorny NoahGorny force-pushed the pip-get-all-versions-of-package branch from 214345d to 2039cfa Compare June 11, 2021 09:36
@NoahGorny
Copy link
Contributor Author

Thanks for the review @uranusjr and @pradyunsg, I have applied your suggestions

@NoahGorny NoahGorny force-pushed the pip-get-all-versions-of-package branch from 2039cfa to 25ffadf Compare June 11, 2021 09:45
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

I think this is good to merge, and we can iterate on this as we go! :)

I'll defer to @uranusjr for the merge.

@uranusjr uranusjr changed the title Pip search get all versions of a package Implement new command 'pip index versions' Jun 11, 2021
@uranusjr uranusjr merged commit 3751878 into pypa:main Jun 11, 2021
@uranusjr
Copy link
Member

I created #10052 to track the follow-up messaging improvement I mentioned earlier.

@NoahGorny NoahGorny deleted the pip-get-all-versions-of-package branch June 11, 2021 11:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2021
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.

10 participants