-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Document the way pip chooses what version to install more explicitly #8117
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This sentence feels so confusing. I can instinctively understand the implication, but feel it is not explaining it right, but can’t point my finger to where nor suggestion something better 😞
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.
Agreed, it's pretty bad. I need to rethink this (at least part of the problem is the fact that
--upgrade
doesn't do what I (and I suspect most people) think it should.I'll have another go at the text here.
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.
TBH as an user after reading this I have no idea what
--upgrade
is supposed to do anymore. With the legacy resolver, upgrading everything as a whole was not possible and I think the flag was to enable users to resolve dependencies by themselves and decide which package to upgrade (i.e.pip install -U this
is equivalent toupgrade this
in other package managers). With the new resolver, it might be less mind-boggling for users if we havepip upgrade
, and just let the resolver tries its best during installation.Since
--upgrade
doesn't really preserve backward-compatibility (it would if--upgrade-strategy=eager
is provided IIUC), IMHO if when the new resolver becomes default andupgrade
subcommand is yet to be ready, we may want to flag it as deprecation (e.g. *--upgrade
may not do what you want, since the dep-resolve process influences the choice of which package to be upgraded/downgraded). If one wants some sort of a new version, perse can always specify it explicitly, which also complies with the Zen of Python.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.
I think this is the correct mental model both with the legacy and new resolvers. But the problem is in the details. What does
upgrade this
actually mean? Does it only upgeadethis
? Does it upgradethis
and all its dependencies? What do “dependencies” even mean? This fries the brain if you really try to think it through.The operation “upgrade” actually works in a different abstraction level from package installation, and most package managers implement it as such: in a different abstraction level with the manifest/lock mechanism. Having an upgrade operation in the abstraction level pip operates in is quirky by itself, so the definition has to be quirky as a result.
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.
TBH after reading the code I lost all sense of what
--upgrade
does, so I'm glad that I captured my understanding so accurately 🙂More seriously, yes, once you start digging pip's upgrade operation is a very weird beast and it's not at all what youthink it is at first glance. One thing that became obvious to me, for example, while writing tests for the new upgrade strategy code, is that upgrade doesn't even have any meaning unless you factor in the concept of "a new version has been released" and that is an idea that's never really relevant anywhere else in pip.
My feeling is that there's a fairly significant restructuring that needs to be done here before it'll be possible to describe things really accurately. What I'm not sure about is whether we'll be able to get a better description before we do that, or whether we'd be better ignoring the user documentation problem for now and moving this description into the internals documentation (where it can afford to stay a bit confusing...).
But thanks for all the thoughtful responses, I'll let the discussion continue for a while and then read it all up at once, rather than keep responding bit-by-bit like this.