Skip to content

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

@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
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.

@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"

@astrofrog
Copy link
Owner Author

astrofrog commented Nov 22, 2024

@warsawnv @DEKHTIARJonathan @vyasr - thanks for the detailed reviews!

I have just pushed some changes - the main ones are:

  • Added specific examples of multiple frontends/backends
  • Normalized section names to match those defined in PEP 12 and re-ordered a couple of sections as a result.
  • Added Rationale, Security Implications, and Copyright sections
  • Add a specification for [project] metadata
  • Added a sub-section on 'Installing without default extras' in the specification
  • Added a note that we are not addressing the problem of conflicting extras

There are some smaller changes motivated by comments in the reviews above. I believe all comments have been addressed or resolved above, but let me know if you feel I missed anything!

@astrofrog
Copy link
Owner Author

I've updated the preview at https://astrofrog.github.io/peps/pep-9999/

Copy link

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I left a number of small changes to improve wording and style, but overall the content looks great to me now. Sorry that it took me so long to get around to a re-review.

==========

Various use cases for default extras and possible solutions in this PEP were discussed
extensively on `this DPO thread <https://discuss.python.org/t/adding-a-default-extra-require-environment/4898/38>`__.
Copy link

Choose a reason for hiding this comment

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

Is it worth listing other potential use cases without the same level of elaboration as the two below, or do you think that would simply muddle the proposal without adding any value?

Copy link
Owner Author

@astrofrog astrofrog Jan 10, 2025

Choose a reason for hiding this comment

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

I think that the two use cases I mentioned actually cover most of the ones I saw in the various discussions - the second discussion on multiple front-ends or backends is broad enough to cover several cases that were mentioned such as having different computational backends, or different GUI frameworks and so on. I have reworded this to say:

Various use cases for default extras and possible solutions in this PEP were discussed extensively on this DPO thread. These fall into two broad cases that that provide the motivation for the present PEP.

to not make it sound like I just cherry picked two use cases and ignored others. The two use cases here are a synthesis of the different cases I saw in the discussion. However if anyone would like to bring up another distinct use case, I'd be happy to add it!

Comment on lines +79 to +83
as well as in numerous issues and pull requests. The solution that will be
presented below may not be everyone's preferred approach from a philosophical
point of view, however it appears to be the best compromise as it is the only
solution to a very real problem that would not break backward-compatibility of
packaging infrastructure in any significant way. It is an opt-in solution which
Copy link

Choose a reason for hiding this comment

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

I haven't been involved in the prior conversations so perhaps I didn't see the discussions that led to this acknowledgment, but naively it doesn't seem like it adds much here. If you're concerned about getting philosophical pushback from particular reviewers though feel free to keep it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Let's just say there has been some philosophical pushback 😅 - obviously I'd be happy to remove this if people object to including it, but until we get to the point where we have some kind of consensus, it might be safer to keep.

@astrofrog
Copy link
Owner Author

@vyasr - thank you for the review!

@astrofrog
Copy link
Owner Author

I have updated the preview for anyone wishing to read over this now: https://astrofrog.github.io/peps/pep-9999/

Copy link

@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.

A few comments. Let's file a PR against python/peps once these are resolved -- let's get to a d.p.o thread soon. :)

.. code-block:: toml

[project]
default-optional-dependencies = [

Choose a reason for hiding this comment

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

I'm not a huge fan of this name, since this is listing the keys under optional-dependencies rather than PEP 508 strings. OTOH, I think we can pick the bike shed color in the d.p.o. thread. :)

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 see what you mean - I have no strong opinion here and happy to update it if someone comes up with a better suggestion.

How to teach this
=================

The simple rule above regarding only installing default extras when no extras

Choose a reason for hiding this comment

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

Suggestion: Follow the guidance in https://justsimply.dev and removing the use of "just" / "simple" and similar words from this entire PEP.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done, I have checked for these and other similar words

Comment on lines +392 to +393
Adding a special entry in ``extras_require``
--------------------------------------------

Choose a reason for hiding this comment

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

I think this section should cover why "default" is not a good name for the special entry 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.

I have made a tweak to this section to change:

In addition, no other string can be used as a special
string since all strings that would be a backward-compatible valid extras name
may already be used in existing packages.

to

In addition, no other string (such as 'default') can be used as a special
string since all strings that would be a backward-compatible valid extras name
may already be used in existing packages.

Hopefully this addresses your comment?

Copy link

@pradyunsg pradyunsg Jan 10, 2025

Choose a reason for hiding this comment

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

(I can't click the resolve button) Yes. :)

@astrofrog
Copy link
Owner Author

@pradyunsg - thank you for the review and for officially agreeing to sponsor this! Could you confirm once you are happy that your comments are addressed, and I can go ahead and open a PR to the main PEP repo?

One small technicality I don't quite understand - in the PEP submission guidelines it says that I should pick a number for the PEP when submitting the PR which is neither used in the repo or in an existing open PR. However, it also says 'Once approved, they will assign your PEP a number' (about the PEP editors). Is it that I should attempt to pick a valid number and that after their review process they confirm whether the number is ok or whether to use another one?

@astrofrog
Copy link
Owner Author

astrofrog commented Jan 10, 2025

Another question is whether you think I should open PRs with a reference implementation and adding the links here before opening the PR to the main PEP repo, or whether it is premature to do this? (I have a WIP branch getting this to work with flit and pip)

@pradyunsg
Copy link

pradyunsg commented Jan 10, 2025

Is it that I should attempt to pick a valid number and that after their review process they confirm whether the number is ok or whether to use another one?

Yup, you can pick the next available PEP number (currently 790, I think?) and if two PRs use the same number for different content, the PEP editors will provide guidance to resolve the conflict. :)

Another question is whether you think I should open PRs with a reference implementation and adding the links here before opening the PR to the main PEP repo, or whether it is premature to do this?

Neat! I'd say it'd be best link to your branches from the initial PEP draft, that is submitted to the main PEPs repo. It doesn't need to be a PR to flit/pip right now, and a PoC "look, it works" would be better for initiating the discussion (I'm partly concerned about the discussion spreading across to happen on the PRs rather than just the d.p.o thread).

I can go ahead and open a PR to the main PEP repo?

Yup! Also, please go ahead and add links to the draft implementations in pip/flit, if you're OK with the approach suggested above before filing the PR.

@astrofrog
Copy link
Owner Author

@pradyunsg - thanks! That all sounds good 😄

@astrofrog
Copy link
Owner Author

@pradyunsg - I've now opened a PR to the PEP repository in python#4198

Thanks everyone for all your comments and helping make it to this stage!

@astrofrog
Copy link
Owner Author

If anyone is interested, I've now populated the reference implementation section with links to flit/pip forks which seem to work.

@DEKHTIARJonathan
Copy link

@astrofrog can we make the reference implementation installation : pip install pep-771 ?

@astrofrog
Copy link
Owner Author

Well the implementation needs forks of pip and flit so I'm not sure if we can make it easily installable like this?

@DEKHTIARJonathan
Copy link

@astrofrog it's possible I'm working on it. Gimme a few days

@astrofrog
Copy link
Owner Author

I am going to close this as the PEP is now published (and undergoing some updates in a separate PR). Thanks everyone for the discussion on here!

@astrofrog astrofrog closed this Feb 5, 2025
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.