Pass Install Extras to Markers

[reesehyde]

Pull Request Check List

Resolves: #834
Improves: #6409
Resolves: #6419
Resolves: #7748
Resolves: #9537

This PR exposes the extras supplied at install to the 'extra' marker for dependencies, taking up @radoering's offer to create a PR implementing this functionality to allow switching torch installation versions based on a cuda extra.

It is a duplicate of the feature in python-poetry/poetry-core#613 but it appears that PR is no longer active, with the last updates in August 2023 and an unanswered question about timeline from March 2024. This PR takes a different approach: I noticed that python-poetry/poetry-core#636 added special handling for the extras marker value but that marker isn't populated often, so I opted to populate the value when extras are specified in the installer.

See issues and unit tests for more details but the idea is to, among other things, enable exclusive extras like so:

[tool.poetry]
package-mode = false

[tool.poetry.dependencies]
python = "^3.10"
click = [
    { markers = "extra == 'one' and extra != 'two' and extra != 'three'", version = "8.1.1", optional = true},
    { markers = "extra != 'one' and extra == 'two' and extra != 'three'", version = "8.1.2", optional = true},
    { markers = "extra != 'one' and extra != 'two' and extra == 'three'", version = "8.1.3", optional = true}
 ]

[tool.poetry.extras]
one = ["click"]
two = ["click"]
three = ["click"]

@radoering it looks like you're very familiar with this issue, would you be the right person to review? 🙏
I'm sure I'm missing something here but look forward to iterating!

Edit by @radoering: fix links to poetry-core PRs

• Added tests for changed code.
• Updated documentation for changed code.

[radoering]

I will try to take a closer look at the end of the week.

[radoering]

This looks really good. Nice work. 👍

It looks like some of the changes are not tested yet or may be redundant. See separate comments for details.

[reesehyde]

Thank you so much for the great feedback here! All the comments make sense to me, I'm working through it and will submit updates and responses later this week

[GeeCastro]

How would this differentiate between the extras for the project itself and the extras for a dependency package?

1. For the project the scenario would be "I want to install my project with cuda124 extra" which I guess you could interface with the -E/--extra option

poetry install -E cuda124

and define the in the package

[tool.poetry.extras]
cuda124 = ["torch"]

Here your suggestion would add

[tool.poetry.dependencies]
python = "^3.10"
torch = [
    { markers = "extra == 'cuda124'", version = "2.4.0+cu124", optional = true},
                ^^^^^^

2. And a package-specific extra for example

poetry add "pandas[aws]"

which would show in the extras field as follows

[tool.poetry.dependencies]
python = "^3.11"
pandas = {extras = ["aws"], version = "^2.2.2"}
          ^^^^^

Would the differentiation be made on the following?

1. project extras in the marker string
2. package's extras in the extras field

If so, I must have misunderstood the docs because I thought these 2 syntaces express the same requirements/constraints

[reesehyde]

I apologize for the long delay here, I had to pause this before I managed to finish all the updates. But it hasn't left my mind, and I'll be able to resume the updates in a couple weeks and will push them up and respond to everything in mid-September! Thanks for the patience

[mg515]

@reesehyde Any update on this? Would love to have this feature available. ^^

[reesehyde]

I apologize for the long delay here, I've been working on the suggested updates but uncovered an issue in the process. I've added a couple breaking tests which will hopefully help illuminate it. But I'm a little stuck on the best course of action, so would love a push in the right direction if you're able @radoering.

It seems that part of the reason the initial tests passed is because they used conflicting versions of torch, which works only because they then go through the process of merging duplicates since the package name is the same in all cases.

However, dependencies with their own differing extras (e.g. torch[cpu] and torch[cuda]) are not considered duplicates, so they result in a SolverProblemError.
Simply considering them duplicates creates its own issues (e.g. see the test_solver_resolves_duplicate_dependency_in_extra test). So we need some way to do overlapping marker resolution in these cases without breaking other solving cases.

Another possibility would be scoping this PR down to not support conflicting dependency extras, but I'm not certain how that would look.

[reesehyde]

How would this differentiate between the extras for the project itself and the extras for a dependency package?

@GeeCastro this is a great question, you're right that they are basically the same syntax! The problem comes up when you have conflicting extras, e.g. if the cuda extra requires torch == 1.0.0+cuda while the cpu extra requires torch == 1.0.0+cpu. Packages must be resolvable with every possible combination of extras, so if you tried to use both (--extra cpu --extra cuda) it would be impossible since 1.0.0+cuda conflicts with 1.0.0+cpu. Markers provide a way to say "cuda extra requires torch == 1.0.0+cuda, but only if the cpu extra isn't also present" so they don't conflict.

[reesehyde]

@radoering thank you again for all the help here! I've pushed updates, responded to all threads, and rebased on main.

I believe everything has test coverage now, there are just small open questions on:

1. Whether to leave redundant changes in both places or isolate to one place
   • Either the L465 change or L558 change are sufficient to pass the new test_installer tests, but not sure if it's worth including both for "marker value correctness" or in cases where having both may shorten the solving steps required.
2. Whether to leave or remove the changes and test which rely on root extra dependencies having the extra marker value even though it's not generally populated in those cases
   • I added test coverage for the L623 change but you mentioned that test scenario may be unrealistic

Edit: The bug referenced was still present, but I added an improved test for it and finally found a fix. Final open question is:

3. A review of that new logic

[github-actions]

Deploy preview for website ready!

✅ Preview
https://website-6n69zvetr-python-poetry.vercel.app

Built with commit cc566ea.
This pull request is being automatically deployed with vercel-action

[radoering]

Trying out the examples, I unfortunately discovered some issues:

1. tool.poetry.extras will be deprecated in favor of project.optional-dependencies in the next version. I updated the examples to the new syntax and thereby noticed a bug in poetry-core, which is especially relevant for custom sources: PEP 621 support: fix merging project.optional-dependencies with tool.poetry.dependencies poetry-core#792
   I added a commit to temporarily use my branch with the fix here.
2. In general, the examples did only work for me when setting poetry config installer.re-resolve false. Without this setting I always get solver issues when trying to install the examples.

I did not find a solution for the second issue. In general, the issue is that on the one hand we want to filter unused extras in Provider to avoid conflicts but on the other hand we do not want to filter them to uninstall unwanted extras when calculating the operations in Transaction.

Unfortunately, poetry config installer.re-resolve false will not be the default in the next version (but may become the default in long-term) so I am wondering if we just add a warning in the docs that exclusive extras will only work with poetry config installer.re-resolve false?

[reesehyde]

Thanks for finding and fixing that bug @radoering!

In general, the examples did only work for me when setting poetry config installer.re-resolve false. Without this setting I always get solver issues when trying to install the examples.

On my investigation here I think the only thing that doesn't work are conflicts between "no extras" and "some extra" when installing the "no extras" case with re-resolve. For example, this can fail:

[tool.poetry]
package-mode = false

[project]
name = "torch-test"
dependencies = [
    "torch (==2.3.1+cpu) ; extra != 'cuda'",
]

[project.optional-dependencies]
cuda = [
    "torch (==2.3.1+cu118)",
]

[tool.poetry.dependencies]
python = "^3.10"
torch = [
    { markers = "extra != 'cuda'", source = "pytorch-cpu"},
    { markers = "extra == 'cuda'", source = "pytorch-cuda"},
 ]

[[tool.poetry.source]]
name = "pytorch-cpu"
url = "https://download.pytorch.org/whl/cpu"
priority = "explicit"

[[tool.poetry.source]]
name = "pytorch-cuda"
url = "https://download.pytorch.org/whl/cu118"
priority = "explicit"

[build-system]
requires = ["poetry-core>=2.0.0"]
build-backend = "poetry.core.masonry.api"

When installing with no extras and re-resolve = true we get:

Incompatible constraints in requirements of torch-test (0):
torch (==2.3.1+cu118) ; extra == "cuda" ; source=pytorch-cuda
torch (==2.3.1+cpu) ; extra != "cuda" ; source=pytorch-cpu

However it works with --extras cuda or with re-resolve = false.

But I think importantly the workaround is just to use exclusive extras, rather than the cuda extra that conflicts with the base. i.e. this works even with re-resolve = true with no extras, cuda, and cpu extras:

[tool.poetry]
package-mode = false

[project]
name = "torch-test"

[project.optional-dependencies]
cpu = [
    "torch (==2.3.1+cpu)",
]
cuda = [
    "torch (==2.3.1+cu118)",
]

[tool.poetry.dependencies]
python = "^3.10"
torch = [
    { markers = "extra == 'cpu' and extra != 'cuda'", source = "pytorch-cpu"},
    { markers = "extra != 'cpu' and extra == 'cuda'", source = "pytorch-cuda"},
 ]

[[tool.poetry.source]]
name = "pytorch-cpu"
url = "https://download.pytorch.org/whl/cpu"
priority = "explicit"

[[tool.poetry.source]]
name = "pytorch-cuda"
url = "https://download.pytorch.org/whl/cu118"
priority = "explicit"

[build-system]
requires = ["poetry-core>=2.0.0"]
build-backend = "poetry.core.masonry.api"

This would also explain why all the tests succeed even with your new config_installer_reresolve auto-use fixture, as there is no "extra conflicts with non-extra" case tested.

Updated the example, let me know what you think.

[radoering]

Thanks for the update @reesehyde!

I did not realize that the second example will work if I leave out the extra != ... in project.optional-dependencies.

I updated the docs with your updated examples (with some minor adaptions):

• build-system is not required with package-mode = false
• tool.poetry.dependencies.python can be replaced with project.requires-python
• I put tool.poetry after project

Further, I added a warning that the first example will only work with installer.re-resolve set to false.

[reesehyde]

Awesome, thanks so so much for all the help over these past months @radoering! 🙏

I may look more into whether it's possible to support re-resolve = true in a future PR but agree it's not worth holding this one up for it. All those changes look good to me, let me know whether we need anything else or this is is ready to merge

[radoering]

I have tried everything so far and skimmed over the code again. I think it is ready to merge now. Thanks @reesehyde for your persistent work on this topic.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.