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

Draft review for default extras PEP #1

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Conversation

astrofrog
Copy link
Owner

@astrofrog astrofrog commented Sep 25, 2024

This is a PR internal to this fork for initial collaboration on the PEP - leave comments/suggestions or open PRs to my branch to suggest changes/additions!

Preview: https://astrofrog.github.io/peps/pep-9999/

Motivating discussion: https://discuss.python.org/t/adding-a-default-extra-require-environment/4898/152

peps/pep-tbd.rst Outdated
**Invalid examples:**

* ``package[pdf, -pdf]`` - an extra cannot be both negated and non-negated.
* ``package[-nondefault]`` - if ``nondefault`` is not specified as a default extra.
Copy link

Choose a reason for hiding this comment

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

This is problematic, because it's not possible to determine if it's invalid without obtaining the Default-Extras metadata value (which in the case of a sdist, could require building the package).

It would be much better to say that this is valid, and define appropriate semantics for it (probably just "attempting to remove an extra that isn't defined, or isn't in the list of default extras, does nothing").

Copy link
Owner Author

Choose a reason for hiding this comment

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

I have updated this

peps/pep-tbd.rst Outdated

**Invalid examples:**

* ``package[pdf, -pdf]`` - an extra cannot be both negated and non-negated.
Copy link

Choose a reason for hiding this comment

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

Why make this invalid? If the user tries to install two packages, one of which depends on package[pdf] and the other depends on package[-pdf], then you'll have effectively the same situation to deal with, and you won't be able to tell in advance. So you need to decide what the semantics are when that happens, and when you do, it won't be a problem to apply that same semantics to package[pdf, -pdf].

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes I agree with you, I have updated this to make it clear that the non-negated extra should always take precedence, and also that - does not mean a requirement of absence of dependencies, but just that an extra isn't needed (but might still get installed via some other dependency).

@astrofrog
Copy link
Owner Author

astrofrog commented Sep 25, 2024

@pfmoore - thank you for taking a look at this already! I'll have a think about the two comments you left, and I was curious if you had any thoughts on the second alternative I just added in my last commit? Essentially I am wondering if we should leave it up to tools such as pip to provide a way to ignore default extras (when they also add support for supporting the default extras), as this would avoid the complexity of adding a new syntax in the package specification.

@pfmoore
Copy link

pfmoore commented Sep 25, 2024

Strong -1 on making it something tools have to do. There's already too many options, both in pip and in uv, and making it a tool option makes it impossible for one library to depend on another one without the default extras.

@pfmoore
Copy link

pfmoore commented Sep 25, 2024

To be honest, I think that in its current form, this PEP fails to cover the discussions that happened on the Discourse thread. You'll need to cover all the possibilities raised there before this is ready for submission - at a minimum, putting the various proposals that were made into the "Rejected Alternatives" section, and explaining what is wrong with them and why (in your opinion) your proposal satisfies the relevant use cases better.

@astrofrog
Copy link
Owner Author

@pfmoore - sounds good, now that I have a skeleton I think I'll go through the discussion more systematically. Thanks for this initial review and feedback, it is very helpful!

@astrofrog astrofrog force-pushed the default-extras branch 2 times, most recently from 627233d to e499678 Compare September 25, 2024 13:38
@astrofrog
Copy link
Owner Author

@pfmoore - I have spent a few hours combing through the long discuss.python.org thread as well as several of the GitHub threads referred to, and have now tried to boil it down to the rejected alternatives in the latest version of this PEP. If you have time at some point I would be grateful if you could let me know if this is on the right track in your opinion.

@astrofrog
Copy link
Owner Author

I'm also going to ping @RonnyPfannschmidt, @pradyunsg and @DEKHTIARJonathan as people who were specifically interested in writing/contributing to a PEP - please let me know if you'd like to join as authors, or either way if you have any feedback on this initial draft so far!

peps/pep-tbd.rst Outdated

Example with multiple default extras::

Default-Extras: recommended,pdf

Choose a reason for hiding this comment

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

use multiple headers there instead of csv just like provides extra

peps/pep-tbd.rst Outdated
Disabling all default extras
----------------------------

One idea was to allow a special syntax to disable all default dependencies,

Choose a reason for hiding this comment

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

in my proposal i pososed a special form of package[-, explicit ,list, of, extras] as canonical form

the singular - denoting that default extras ought not to be considered for that particular requirement specifier

@RonnyPfannschmidt
Copy link

a key hinge on moving my proposal forward was how to get backward compatibility with non-default-extra aware installers

multiple libraries i was working on would be "in hells kitchen" if they where just migrated to default dependencies without a way to ensure legacy installers supporting it

@abravalheri
Copy link

It would be interesting to add in the rejected ideas section, why making the extra marker behave more similar to other markers - for example extra != "name" or extra not in ("name", "other") - is not viable.

peps/pep-tbd.rst Outdated

extras_list = (-)?identifier (wsp* ',' wsp* (-)?identifier)*

If both an extra and its negated version appear in an extras list, the

Choose a reason for hiding this comment

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

I think it would be better to disallow this. I expect making either an extra or its negation take precedence would lead to surprises for users.

Choose a reason for hiding this comment

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

Agreed with Ethan

@astrofrog
Copy link
Owner Author

@abravalheri good idea, could you say in a few words why it would not be viable in your opinion?

@abravalheri
Copy link

Hi @astrofrog , I don't know why not. That is why I was asking 😅.

That approach does not allow you to have a negative extra syntax, but allow you to have an extra that when specified exclude dependencies. For example:

dependencies = ["dep; extra != 'minimal']
pip install mypkg[minimal]

which has a very similar overall effect and could be used in similar usecases.

@astrofrog
Copy link
Owner Author

astrofrog commented Sep 25, 2024

@abravalheri - this would be nice but does it actually work for you? I just tried it in a real package and even when specifying [minimal] the dependencies with extra != 'minimal' got installed. But it's possible I'm doing something wrong so would be very interested to hear if you have it working in a real package.

Or are you suggesting that getting that to work somehow would be the alternative solution?

@abravalheri
Copy link

abravalheri commented Sep 25, 2024

Exactly, that does not work now.

But it would be an alternative change (many people do try that approach intuitively and are disappointed by the results), so it would be interesting to understand why one alternative is prefered to other in the rejected ideas section.

@DEKHTIARJonathan
Copy link

DEKHTIARJonathan commented Sep 25, 2024

@astrofrog thanks for starting this, I'll be happy to co-author this with you if you need help and help you find a PEP sponsor if needed.

I think you're taking the requirement of this PEP a bit too far and it can be "greatly reduced" and consequently improved in its likelihood to be merged.

I would summarize the proposal as follows:

  1. We don't need a "special magic no-extra" dictionary key:

    1. Either use "" or None, consequently package installers like PIP can do: if extra_requires is None or extra_requires == ""
      This has the benefit to not change in any shape of way the specification of the METADATA file and make it fully backward compatible (to be noted: only updated "installers" can pick up the default extra).

    2. I would entirely remove this feature of -extra, extra-dependency sets are cumulative not subtractive. If you want to introduce this feature, please open a new PEP dedicated to this. It's out of scope. And frankly can introduce a crazy quantity of edge cases as @pfmoore highlighted.

In my opinion, keep it to the "core of the proposal" => providing a set of "optional dependencies" by default, unless some other set is required.

CC: @ethanhs-nv

@astrofrog
Copy link
Owner Author

astrofrog commented Sep 25, 2024

@DEKHTIARJonathan - thanks for the feedback! Note that currently the meat of the proposal here is actually quite concise and many of the sections (including the empty string extras_require) are in rejected alternatives (note that None is also not an option as in the package metadata there is no difference between Python None and string None, and someone somewhere may have used None already as a valid extras name). The approach here (basically defining Default-Extras and defining a - syntax in extras lists) seemed to be the broad consensus of the discuss thread?

@DEKHTIARJonathan
Copy link

@astrofrog I think your example can be improved package[recommended].

I think we have a few much better use-cases: backends.
- graphic backend: tkinter, etc.
- compute backend: cpu/gpu, numpy vs cupy
- communication backend: etc.
etc.

These are prime examples of this "scenario" and probably the most interested parties.

A default "lightweight - simple" backend unless you ask for a backend larger/more specific.

@astrofrog
Copy link
Owner Author

a key hinge on moving my proposal forward was how to get backward compatibility with non-default-extra aware installers

multiple libraries i was working on would be "in hells kitchen" if they where just migrated to default dependencies without a way to ensure legacy installers supporting it

@RonnyPfannschmidt - is it correct that legacy installers would be fine with Default-Extras being present in the metadata since they would just ignore it? Are you referring to the fact that legacy installers would break if anyone used the - syntax in any of the dependencies? (for example if a package started declaring one of its dependencies as package[-pdf]?)

If it is correct that legacy installers would be fine and ignore Default-Extras, then maybe we should make it clear in the PEP that packages that start using the - syntax in their own dependencies will be implicitly breaking compatibility with legacy versions of e.g. pip? I see this more as a feature that would be useful for end users who are installing packages to choose what they want rather than packages specifying dependencies, though of course we can't stop the - feature being used in the latter.

@astrofrog
Copy link
Owner Author

Exactly, that does not work now.

But it would be an alternative change (many people do try that approach intuitively and are disappointed by the results), so it would be interesting to understand why one alternative is prefered to other in the rejected ideas section.

Ok sounds good, I'll try and write something up about this once I've had a chance to think about whether/why we can't do it

@astrofrog
Copy link
Owner Author

astrofrog commented Sep 25, 2024

@RonnyPfannschmidt - I also wanted to say that if you have an advanced draft that you think would be a better starting point than the one here, I'd be happy to switch over, so just let me know! (EDIT: just found RonnyPfannschmidt#1, sorry I missed this somehow!)

@DEKHTIARJonathan
Copy link

DEKHTIARJonathan commented Sep 25, 2024

The approach here (basically defining Default-Extras and defining a - syntax in extras lists) seemed to be the broad consensus of the discuss thread?

No I absolutely don't think it was a consensus. It's what 1 person ( Pradyun ) mentioned. Then we brainstormed a bit about the idea. However this is an entirely new idea, syntax and concept. To me it doesn't have it's place here, it's ortogonal to "having default extra": one can live without the other.

This specific point adds a lot of complexity to the entire proposal, is far from being intuitive and introduce MANY corner cases. I'm not in favor in adding it here.

I think the proposal should be simple:

A. If no extra is demanded - use the default set of extra (either by picking None, "", or Default-Extras: (my least favorite since it forces to change the METADATA file standard).
pip install my_pkg => gets the default extra

B. If any extra is demanded => ignore the default, behave exactly like today
pip install my_pkg[A,B] => gets extras A and B - default is ignored

C. We could consider adding pip install my_pkg[] or pip install --no-default-extra my_pkg => gets no extra at all including the default (essentially today). I'm not sure if there's a valid use-case for that, but if one is presented we could consider this

@astrofrog
Copy link
Owner Author

@DEKHTIARJonathan - if we introduce a way to have default extras, we probably also need to provide a way to opt out of these, right? Otherwise aren't these just equivalent to moving the dependencies to install_requires?

@DEKHTIARJonathan
Copy link

DEKHTIARJonathan commented Sep 25, 2024

@astrofrog

if we introduce a way to have default extras, we probably also need to provide a way to opt out of these, right? Otherwise aren't these just equivalent to moving the dependencies to install_requires?

No that's not how it should work.

If you buy a car which by default is white.

  • Either you don't specify the color and get a white car
  • You specify you want your car red and get the car red. (Not white and red, not -white and +red)

When in python you do

def say_hello(name="Jack"):
    print(f"Hello {name}")

You don't "remove the default value" and add the value you want. The default value is only selected if you don't provide one.

I don't see why this should be any different. It's a "default extra-require" not a "mandatory-that-you-can-remove"

peps/pep-tbd.rst Outdated
Comment on lines 25 to 36
Package maintainers often use extras to declare optional dependencies that
extend the functionality or performance of a package. In some cases, it can be
difficult to determine which dependencies should be required and which should be
categorized as extras. A balance must be struck between the needs of typical
users (who may prefer most features to be available 'by default') and users who
want minimal installations without large, optional dependencies. One current
solution is to define an extra called, for example, ``recommended``, which
includes all non-essential but suggested dependencies. Users are then told to
install the package using ``package[recommended]``, while those who prefer more
control can simply use ``package``. However, in practice, many users are unaware
of the ``[recommended]`` syntax, placing the burden on them to know this for a
typical installation.

Choose a reason for hiding this comment

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

Just wanted to comment with some kudos: this is a really well written summary of why having the install 'by default' include more dependencies than one including an explicit extra, nice!

This is exactly how I'd use this PEP: to provide a 'by default' installation that includes multiple optional-but-usually-helpful dependencies, but still allow a more advanced 'minimal' install that excluded those.

Thanks for picking this up after a couple years of commentary 👍

@pfmoore
Copy link

pfmoore commented Oct 15, 2024

@pfmoore: does that counter-argument make sense to you?

To be clear, I have no direct need for any of this. I'm simply reporting points that were made by others in the Discourse thread1. The important thing is that somebody asks the people who originally requested this capability whether it satisfies their use cases. Everyone on this issue may think that the workaround of an "empty" extra is sufficient, but that's irrelevant if project maintainers and/or users hate the idea.

That's part of the job of creating a PEP - getting buy-in from the users affected by the proposal.

Footnotes

  1. And I may be misremembering things...

@NevilleS
Copy link

NevilleS commented Oct 15, 2024

OK, thanks @pfmoore: I misread your comment as-if you could represent the "others" here 😄, that's fair!

In that case, I think the right next step would be to amend the PEP to simplify it down to just the Default-Extra proposal and remove the language around deselecting extras (ie Option 2) and share that back with the Discourse thread. Personally I think that strikes a reasonable balance: it allows package maintainers to opt-into a new default for their project, but those who prefer the current style of "no extras" being the default can decide not to use it.

@astrofrog sorry if I'm not being helpful 🙃

@astrofrog
Copy link
Owner Author

@NevilleS you are definitely being helpful, thank you! I will try and tidy things up shortly 😀

@astrofrog
Copy link
Owner Author

@NevilleS - I have removed option 1 and updated option 2, to now include some specific example applications of the PEP, as I think this will be important for convincing maintainers that this is versatile and should solve several different real-life use cases. Would you be happy to take a look at the PEP as it stands to see if you are happy with it?

@DEKHTIARJonathan - as a co-author, could you now read through carefully to make sure you are happy with it and make any suggestions if needed? Note that the 'meat' of the proposal is very simple as you were saying - the PEP is just long-ish because it's important to mention the rejected alternatives and also give some real-life applications of it.

I am also going to send an update to the thread to solicit feedback from other people who were originally involved in the conversation.

@astrofrog
Copy link
Owner Author

Also @pradyunsg would you be able to take a look at check that you would be happy with this approach and likely to sponsor it once we have converged?

@astrofrog astrofrog marked this pull request as ready for review October 16, 2024 10:19
@astrofrog
Copy link
Owner Author

If anyone wants to see the rendered version, it is at https://astrofrog.github.io/peps/pep-9999/

@hamogu
Copy link

hamogu commented Oct 17, 2024

I don't have any specific comment on how exactly this is implemented, but I want to just comment that the general concept would have been very helpful for me a while ago.

I have a package with some somewhat esoteric dependencies for a graphics backend. I'd love to make all the packages needed for that backend the default so that "it just works" for all users, but I can't because that causes complications for CI and for those very few users who cannot match the requirements for that backend because some of its dependencies is not available for e.g. windows.

Currently, I've solved the problem by re-doing the implementation of graphic output to rely on some other graphics package (which I can add to "minimal" because it's available everywhere), but it's very annoying that essentially the limitations of pip determine which graphics package I can and cannot use as the default.

making one of the backends or frontends be always required, or requiring users
to always specify extras, e.g. ``package[backend]``. Having a way to specify one
or more default backend or frontend and providing a way to override these
defaults would provide a much better experience for users.

Choose a reason for hiding this comment

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

I read this yesterday and couldn't think of an actual package that does this. I just found one today; viscm needs to be installed with either extra PyQt or Pyside, but isn't functional on its own. Just in case you need a concrete example here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks! Same with glue - I wasn't sure if I should single out specific packages in a PEP though?

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 (especially if you can give more than one illustration); I believe I've seen it done in other packaging-related PEPs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants