-
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
Implement a --group
option for installing from [dependency-groups]
found in pyproject.toml
files
#13065
base: main
Are you sure you want to change the base?
Conversation
Steps taken: - add `dependency-groups==1.3.0` to vendor.txt - add dependency-groups to vendor __init__.py - run vendoring sync - examine results to confirm apparent correctness (rewritten tomli imports)
`--group` is supported on `download` and `install` commands. The option is parsed into the more verbose and explicit `dependency_groups` name on the parsed args. Both of these commands invoke the same processor for resolving dependency groups, which loads `pyproject.toml` and resolves the list of provided groups against the `[dependency-groups]` table. A small alteration is made to `pip wheel` to initialize `dependency_groups = []`, as this allows for some lower-level consistency in the handling of the commands.
A new unit test module is added for parsing dependency groups and used to verify all of the pip-defined behaviors for handling dependency-groups. In one path, the underlying exception message from `dependency-groups` is exposed to users, where it should offer some explanation of why parsing failed, and this is therefore tested. Some related changes are applied to the dependency groups usage sites in the src tree. The signature of the dependency group requirement parse function is simplified, and its usage is therefore updated. A bugfix is applied to add a missing `f` on an intended f-string.
This initial suite of tests is modeled fairly closely on existing tests for requirements files. Tests cover the following cases: - installing an empty dependency group (and nothing else) - installing from a simple / straightforward group - installing from multiple groups in a single command - normalizing names from the CLI and pyproject.toml to match - applying a constraints file to a dependency-group install
Exciting to see this up!
To clarify for those that have not read the discussion, I feel like we did not reach consensus that automatically discovering a I'll disclaim that I am one of the people opposed, but so was @pfmoore — I feel like this merits further discussion.
As a minor note, we have not added dependency group support to the |
I agree with that assessment. I'm proposing it as an initial option, but am 100% ready to adjust course if there's an alternative with pip maintainer support.
Maybe I'm inferring too much, but does this suggest that there's an alternative UX you would find less surprising? I've thought up alternatives, but none of them seem clearly better to me. And 👍 to the note about keeping |
I'm OK with It did occur to me when this PR was submitted that we hadn't reached agreement on auto-discovering I don't know the right answer here. It feels to me like the "traditional" role of pip, as a standalone installer, is being eroded by the ecosystem drift towards the "everything is a project" model. I don't like that, but maybe at some point I have to accept the inevitable and stop blocking things based on an outdated view of what pip is for. I would definitely like to know what the other @pypa/pip-committers think about this, though. There's also a wider discussion about how pip fits into the modern packaging ecosystem that I think the maintainers need to have, but that hasn't happened yet either. I don't want to block things on the basis of some grand philosophical debate, so if someone says "let's just do it for now and worry about the bigger picture later", I'm OK with that. |
What about adding a FWIW, I also like Original post:I think users would like for this to work without having to add some new pip install -e. --group test However, then I think most people would expect this to work: pip install . --group test which is a lot more dubious; another clearer example would be pip install ./some/thing ./other/thing --group test Which isn't clear to me at all which of the two directories would be used - or both. Or neither and just the current directory. If there was a pip install -e. --project-dir . --group test But that's fully explicit. I think either defaulting the project-dir to FWIW, I'm pretty happy if this gets in in any state, and I like shorter CLIs if possible. :) |
This was one option I considered, as well as, similar,
I read some of your comments there as weakly supporting the idea of
I do not like the idea that Is this perhaps an argument in favor of accepting the pyproject file (or dir?) as an option? Any user who asked "why doesn't I'm much more comfortable with the idea of adding an option for the A file option keeps A dir option adds the notion that there is a "project directory", and I'm much less clear on what that means. It would only drive |
I made two concrete recommendations in the issue (both of which are similar to @henryiii's suggestions here):
For discussion purposes, let's list a couple more proposals:
I don't want to just repeat the discussion from there — I think there's a fair bit of context on the upsides and downsides to each of those in the thread already. Unfortunately, I don't think there's an obviously superior option here. I'll try to summarize some thoughts briefly: (1) is very explicit which is good for teaching but can be repetitive and verbose Since @sirosen replied while I was authoring this..
The |
Fascinating discussion. I am also ok for accepting any option, but I think the best way will be to use Just to make a discussion more concrete - (and provide context) we are eyeing into refactoring airflow monorepo for Airflow 3 (it's partially done already but we are also waiting for this one to land), and we will have 90+ sub-projects eventually in Airflow monorepo (yep, I know it's crazy). We are now recommending So - for multiple reasons we make sure in our docs and workflows that both In our case we currently have (this wil change and improve but here it is):
eventually it will be:
What I really would like with groups is:
Generally i'd be for not having In
|
Weakly in the sense of "yeah, I guess we might have to do that". It has problems, as you mention, though, so I'm definitely not enthusiastic about it 🙁
Yes. And stronger, it's an argument for requiring the
+1. Your arguments make sense to me. (But I'm not convinced about defaulting the
There are two aspects I don't like:
|
I'm a strong -1 on this. There are so many edge cases that don't have intuitive (or in some cases even plausible) interpretations that I don't even think this is the "user friendly" option. It's deceptively straightforward in simple cases, but will end up causing nasty bugs as soon as people do something unusual. I suggest that we simply drop this as a possibility, as it feels like people are only thinking about the "obvious" cases, and I have no appetite for coming up with multiple problematic cases just so that people can suggest workaround after workaround. (For example, if a requirements file includes |
In general, I am only interested in implementing behaviors which are easy to reason about and don't contain avoidable ambiguities. Even within that confined space, we are debating how to create a UX in which most naive users won't be easily confused.
I think this is a good point of concern. A user could do something like...
and expect... ? I'd like to have a solution in which the above usage, or its analogue, emits a warning or error, instructing the user that they're misapplying the options.
I find this convincing in principle, but I worry that it makes for a very verbose CLI experience for interactive usage.
If we go down this path, I have a question: And an initial expectation about requirements and behavior
The check for
This is still on my mind. I agree that it seems redundant, but it also is the most in-keeping with Does it make a difference that the proposed behavior would allow someone to pass something other than
Maybe that's a point against this, but I want to note it and understand if it elicits a strong positive or negative response from anyone. The behavior as implemented today doesn't have to worry about the filename. I was thinking Footnotes
|
I think this rules out (2) — I'm happy not to discuss that option further as suggested by @pfmoore but it's worth reiterating (as you have) that users will expect this and there should be warnings that guide them to the correct behavior.
The idea that this would be allowed is what is most concerning to me about including the filename, especially since it's such a clearly defined standard (a file named As a minor note regarding As I was looking at the CLI to write the above, I realized we recently added |
I know. That's the biggest drawback here. But I'd rather that we're verbose rather than confusing...
I'm against that - while this is important functionality in the wider ecosystem, it's not well aligned to pip's core feature set, and I don't think it deserves one of the limited single-character option names.
No, pip has a general feature that all command line options can be specified in a config file, or via an environment variable. So simply by having the command line option we automatically get the ability to set a default in those ways.
I'm not comfortable about it. But to be fair, the implementation currently allows an invalid |
There's a lot of design discussion going on in this PR that to me seems to boil down to:
If the answer to 1 for this PR is "no", I'd like to point out that it doesn't stop pip adding a concept of a project in the future. If And if the answer to 1 for this PR is "yes", then I'd like people to consider that the answer for 2 could affect a lot more than just this feature, for example if at some point pip reads it's own configuration out of pyproject.toml (i.e. #13003). So I would caution to consider this design quite carefully, looking at what other tools have done here and what challenges they've faced. I would be in favor of something as minimal and unopinionated about user workflows as possible, but I don't have a strong sense for what that is. |
I'm inclined to answer (1) as "no", at least within the scope of this PR. If a narrow change is not possible, I'd rather step back and make a more complete proposal which starts from the notion that
Ah, that explains why I didn't see the kind of env var logic I expected! I noticed the config loading Upcoming updateFor now, I plan to update the PR to implement I'll also validate that the filename provided is Idea:
|
That works for me. The |
I like it too. It also works nice as a building block of our |
I still think pip should accept any filename for the reasons I outlined in #12963 (comment). However given the path is explicit there is the obvious workaround of creating a directory and sticking a custom pyproject.toml there. And nothing stops removing this validation in the future if opinion on this changes. |
Side comment @notatallshaw - this problem in "messy monorepo" could be solved by converting the "custom named" toml files into putting pyproject.toml file for each team in a separate sub-folder of that directory where you currently have custom named files (directories named same way as currrent files). I think that is way better approach - especially that IDEs and such already have "if pyproject.toml" use the right schema etc. |
I personally don't like the "any filename" support, as the PEP specifically is for
I don't mind this, though. While it does allow someone to put a specific file in, it is optimized for the "correct" case following the PEPs. Also, I second that subdirectories with pyproject.toml's would be better, IDEs and validators would like that at least. |
Then we would have a clean monorepo 😛. Sure, that's the goal, but technical nuances and resource capacity have so far got in the way. For fresh projects we do this and can use opinionated tools like poetry, but for the older projects we stick to tools that don't force specific workflows or project structure, and pip has always been excellent in this regard. Anyway, I think I made my point in the linked post, and as long as there is an explicit path there is a workaround. I didn't mean to belabor it here. |
This, to me, is an important reason to include the requirements that the filename is If So the better short-term choice, from a compatibility perspective, is to validate the name. |
Why include the |
It can be |
Personally I think |
I would expect in this hypothetical future that a But I think this discussion would be better broken out in to a "how to make pip project aware" issue. And as I say, I don't want to hold things up bikeshedding, if |
Henry is suggesting an explicit I don't like inferring the |
Oh yeah, maybe to be completely unambiguous: If pip isn't implying the location, the environment, or other flags, I don't have strong opinions (just mild ones I probably shouldn't have mentioned). |
OK, I've been thinking some more about this, and I've come to some conclusions. These are my views, and the other pip maintainers may choose to disagree with me, but I think this is something we need to decide for pip, and as the only maintainer currently active in this discussion, I'm going to take the chance to state my position 🙂 In my view, the key distinguishing feature of pip is precisely that it doesn't mandate any sort of project structure, or workflow, on the user. It's a low-level tool that simply does what the user asks - that comes with some downsides, in that the user must be explicit about what they want, but those are part of the price to be paid for the flexibility pip provides. In particular, pip installs requirements into environments. That's it. We provide a number of ways people can supply a list of requirements1 but ultimately, it's just a list of requirements and a target environment. Users who want a project management tool have a plethora of choices - hatch, PDM, Poetry and uv as well as probably many others. It's a crowded market, and I don't think it's one pip should try to compete in. Our strength has always been, and should remain, as the low-level tool that just does installs, however you want them doing. With that in mind, I'm now a firm -1 on pip ever2 including a concept of the "current project".
Agreed. The above is simply stating my position on the wider picture. I think we're pretty close to a workable solution that doesn't clash with my opinions above.
Agreed. But that's a consequence of the proposed option structure. For me, what's important is that when specifying a dependency group, the user must specify both the group name and the file it comes from. That in itself is a fairly clear and simple statment of what's needed. It's just that options aren't a great way to express two pieces of information at once. See below for an alternative proposal.
And my opinion is the opposite. Having a
I think that's misrepresenting what I said, but my views have been crystallising as part of this discussion, and it's possible I was less definite in previous posts. It's not important enough now that I want to go back and check exactly what I said. But as I said above, I'm now very firmly of the opinion that there will be no future (hypothetical or not) where pip needs or gains a
I'll shut this down explicitly now, just to avoid digressions. My answer to that is "it shouldn't be". No need for an issue (unless you can get another pip maintainer to support a different view, and be willing to argue with me about it 🙂).
It's absolutely not the intention, but there is some merit to the idea that the option for specifying the file a dependency group is to be read from should be clearly part of the dependency group feature. Actually, there's an argument that we shouldn't use two options at all. At the risk of proposing yet another colour for the bikeshed, we could invent a syntax for specifying the group and filename together. Something like One other point I'll make here is around the
Footnotes
|
The fun thing about agreeing with your second sentence is that the comments I might add about the first one become immaterial! I am focusing narrowly on behaviors which "read from a specfiied
👍 And the input on this topic is much appreciated. I think it has been a big help. I'm still attracted to But that leads me to consider...
Speaking from the perspective of someone who has been trying to work out how to implement this, I actually have been starting to think in a similar way. I was maybe shy about saying it before since I thought I was overcomplicating the interface, but this is really a significant simplification for the implementation. For example, consider this usage in one of the proposals: $ export PIP_PYPROJECT_PATH="."
$ pip install -r foo.txt This can't throw an error, even though only "half of the required options were provided". Confusing, right? I agree that any syntax would need to be $ pip install --group "./pyproject.toml: test"
# note, also allows:
$ pip install --group "./foolib/pyproject.toml: build" --group "./barlib/pyproject.toml: build" |
As a minor note, we very briefly discussed including the group and path in a single argument at #12963 (comment). In uv, we report dependency groups as
I'm not really participating in this thread in the interest of uv — I'm trying to help drive a good decision here. |
I don't have any qualms about supporting |
All cards on the table: I'm not sure how I feel about it either. One of the most difficult parts of PEP 735 discussions for me was pushing back against the various requests for "special dependency groups" syntax. I understand the initial attraction of those ideas, but I'm so happy that I stuck to the position that there would be no new or special syntax -- I believe it would have made these sorts of discussions much harder, not easier. So coming back around and proposing a I do like that it would let you pull groups from multiple files, which otherwise is not possible. EDIT: A quick note to clarify for anyone who didn't follow the PEP. People wanted syntax for dependency groups based on package names, project directories, and pyproject.toml files. If we had included a syntax for dependency groups in the spec, it's pretty unlikely that we would have had such perfect foresight that it would have been exactly what we need in this scenario. |
I could live with that, too. The syntax is a lot better than the one I suggested. Honestly, I don't think anything here is ideal syntax, precisely because the whole dependency group feature is a slightly uncomfortable fit with pip's model. So we'll have to compromise somewhere.
Understood, and your input is much appreciated. I wanted to be clear on how this would impact uv (in my view) but I wasn't trying to make any sort of point by doing so. Sorry if it seemed that I was.
I understand that feeling. I'm a little uncomfortable about it myself, too. I do think that having it be a detail of pip's CLI syntax, rather than a packaging standard, is an important difference. While we're mentioning things that make us uncomfortable, it did occur to me that if we made the default for the location of Footnotes
|
script.scratch_path.joinpath("pyproject.toml").write_text( | ||
"""\ | ||
[dependency-groups] | ||
empty = [] | ||
""" | ||
) |
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.
nit: Wrap in a textwrap.dedent?
@@ -38,6 +38,7 @@ class DownloadCommand(RequirementCommand): | |||
def add_options(self) -> None: | |||
self.cmd_opts.add_option(cmdoptions.constraints()) | |||
self.cmd_opts.add_option(cmdoptions.requirements()) | |||
self.cmd_opts.add_option(cmdoptions.dependency_groups()) |
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 didn't include it because I didn't think we'd want the option to be presented for
pip wheel
.
I think we do want it on pip wheel
-- the mental model I have is pip create-wheelhouse
is the longer name for pip wheel
.
Essentially, it's creating a bundle of wheels that you can pass to pip install with the same requirement strings/arguments. And, --group
makes sense within that context IMO.
My feeling on this whole topic is that the quoted blurb describes the bare-minimum that we can reasonably implement here without strong disagreements on this topic, so that we shouldn't change the scope of this PR (to add any project-level concepts or any custom path handling). My 2 cents on this: Let's tackle that whole question in a follow up PR/change/issue. AFAICT, all the proposals at hand are additive (the
Note that longer option names is something we've used in the past to indicate that we want to discourage use of these options (I don't have the discussion handy, but one factoid there was UX research that indicated that longer option names had a strong correlation with lower usage) -- so I'm strongly hesitant on a longer option name here. |
I'm not sure this is true - loading What I want to avoid is having to spend time rehashing this whole debate when someone says that pip should search upward for
Or people do If we can find a way to document that the decision to not try to auto-discover the right |
Yeah the discussion is not about adding more functionality, it's about alternatives to implicit discovery of the |
Right. I started from a low-confidence belief that it was the smallest option with the least disagreement. The discussion here has inclined me to think that it will cause user confusion (even if it's minor) and lead to issues being filled.
My current, pessimistic, belief is that such documentation would not be sufficient. I would expect users to open issues (or just complain in public forums, which is less impactful but no fun) with the notion that "searching up the directory tree would be a small, practical improvement" and suchlike for any other "easy improvement" pip "should" make. I started work on |
Please make sure to test for obvious awkward paths e.g. And I'll just throw another syntax suggestion out there (but I'm not going to advocate for anymore than suggesting it this once):
Apologies, I didn't mean to represent your views, I think I cut out some context attempting to keep some brevity, I should of worded it more like "I agree with not using that flag, I would prefer it to be left for some hypothetical future use if pip ever decides to become project aware" |
It won't stop people asking for enhancements, but it will give us an easy way to reject those requests. "This is by design - see |
Not a problem. We've been exploring a lot of ideas, and my view has changed over the course of the discussion. I simply wanted to clarify that whatever you were referring to wasn't necessarily how I think now. |
By the way, the obvious choice should be considered and rejected if needed, Also, what happens if you want multiple groups? Will the group flag be accepted multiple times, and you would just pass the same path if needed? Unless the |
We already have repo@branch, by the way. IMO that's a potential confusion point if we also had --group name@path, path ~= repo. |
Semi-related, this reminded me of #13062, which is to say it should be clear, if a special syntax is decided on, whether it supports file paths in the |
I appreciate you raising this in a way which is open to rejection... because I'm strongly against it! 😁 I think looking like an extra would be harmful, not helpful. It's not an extra. If a user thinks it's an extra, they're mistaken.
It's a good question. I don't have a working implementation yet, but I do have a clear plan:
I think this will be a rare use-case, since if you control the source you can, like you said, just add aggregate = [{include-group = "part1"}, {include-group = "part2"}] but rare doesn't mean excluded. It will just be a bit more verbose. |
Per review, support on `pip wheel` is desirable. This is net-net simpler, since we don't need any trickery to "dodge" the fact that it is a `RequirementCommand` but wasn't supporting `--group`. The desire to *not* support `--group` here was based on a mistaken idea about what `pip wheel` does.
In discussions about the correct interface for `pip` to use [dependency-groups], no strong consensus arose. However, the option with the most support appears to be to make it possible to pass a file path plus a group name. This change converts the `--group` option to take colon-separated path:groupname pairs, with the path part optional. The CLI parsing code is responsible for handling the syntax and for filling in a default path of `"pyproject.toml"`. If a path is provided, it must have a basename of `pyproject.toml`. Failing to meet this constraint is an error at arg parsing time. The `dependency_groups` usage is updated to create a DependencyGroupResolver per `pyproject.toml` file provided. This ensures that we only parse each file once, and we keep the results of previous resolutions when resolving multiple dependency groups from the same file. (Technically, the implementation is a resolver per path, which is subtly different from per-file, in that it doesn't account for symlinks, hardlinks, etc.)
I want to give a quick recap because the last commit changes the interface being added here. Revised PRAdd a new Usage is as follows:
This option allows users to pass multiple If a path is given but it has any basename other than Implementation DetailsThe CLI parsing is done with The filename check uses Dependency group resolution is done such that each Example UsageBecause the
It's also possible to run against multiple files or to pull a group from just one other file, mixing it with the above syntax. See the following:
Ergonomic Notes
Review Notes
|
--group
option for installing from [dependency-groups]
found in pyproject.toml
in the current working directory--group
option for installing from [dependency-groups]
found in pyproject.toml
files
Update: This PR has undergone a major revision. See this comment for the revised proposal.
The initial comment here is preserved, as it contains useful context and is necessary to understand the following discussion.
This changeset implements
--group
as an option for loading dependency groups, leveraging thedependency-groups
library1.resolves #12963
Example usage
pyproject.toml loading and working dir
Per the discussion in #12963 , this is implemented with support only for loading data out of the
pyproject.toml
file found in the working directory.Although in theory there might be a desire to have any of the following interfaces in the future...
...there is not a clear "winner" amongst these and they are only usable with
--group
.Until or unless there is a more strongly demonstrated need to specify the path in some way, no attempt is made here to support it.2
--group
as the Option Name--group
is known to match the option provided byuv
for installation from groups.There are some lines of argument in favor (e.g., the initial post which mentions this), and some arguments against (e.g., my note that identical option names could incorrectly imply identical behavior).
Ultimately, I chose to propose
--group
because of a UV-independent argument that it is short and declarative.I am open to arguments from
pip
maintainers in favor of other names -- but I think--group
is a good name and that "potential confusion withuv
" is a bad reason not to choose it if it's the best name for the feature.Any alternative name should have some reason that it would also be good for users. e.g.,
--dependency-group GROUP1
could work because it's very explicit.--group
as a repeated option vs comma-delimited--groups
I originally proposed an interface along the lines of
--groups foo,bar,baz
in #12963.I'll just make a quick note that as soon as I started implementing this, I could not see any particularly good reason to add specialized parsing.
Option parsing can collect these for us trivially, and it removes the potential for ambiguity in cases like
--groups foo,bar --groups foo,baz
.Wrapped
dependency-groups
libUse of
dependency-groups
passes through a wrapper module which conforms the errors from that library toInstallationError
.Details of the implementation do (intentionally) leak out, most notably the error strings.
This can be revised if there's a desire for
pip
to control the error messages more tightly.Tests
It was not clear to me exactly how much effort I should invest in unit and functional tests.
I therefore generally tried to follow the guide of "test the subset of features which map to existing tests of
requirements.txt
files".New unit tests and functional tests explore some particular cases, but they are intentionally a little bit limited.
If desirable, we can retest most of the behaviors provided by
dependency_groups.resolve
under the unit tests.Commits & Review
Here's a summary of the (at time of writing) commit series:
--group
--group
supportNotably, the first commit contains the whole vendoring step with no usage. So it should be possible to diff the HEAD of the PR against that commit to see the "real work" here.
Footnotes
As a potentially important aside, I intend to open a thread to move
dependency-groups
intogithub.com/pypa/
for better continuity of ownership and maintenance. ↩Some users will likely be disappointed with this decision. But the nice thing about not deciding anything today is that it can easily be added if a strong argument is made. "No is temporary; yes is forever." ↩