Add support for extras in markers

[BergLucas]

Resolves: python-poetry#834

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

Description

Hello dear maintainers, it's my first contribution to Poetry so I hope I have done the things right.

As you may have seen in the issue mentioned above, there's a feature that's been requested but hasn't yet been implemented. Since I also need this feature, I decided to try to implement it myself and contribute to the project.

In the issue, it is requested that Poetry implements a feature that allows differing sets of dependency extras in the tool.poetry.extras section. I've chosen to make a slightly different implementation that the ones that were proposed because they had some problems in my opinion but this implementation also fix their problems.

In the pyproject.toml, it would look like that :

[tool.poetry.dependencies]
dependency = { version = "...", optional = true }

[tool.poetry.extras]
feature-a = ["dependency[feature-a]"]
feature-b = ["dependency[feature-b]"]

The extras that are located in the tool.poetry.extras section are additional extras that are added to the ones in tool.poetry.dependencies section. So, if there were an extra named feature-c in dependency, installing my_project[feature-a] would install dependency[feature-a,feature-c].

Why this implementation ?

In the 2 implementations proposed in the issue, I thought that there was some problems.

In the one with aliases, it would required many changes to make it work and the dependencies would have been less clear since we would not be able to read the true package names directly.

In the one with the superset that can be filtered, I thought that it would be a good idea at first but on second thought, it was not a really good idea since it would create backward-incompatible changes. Indeed, if we have a pyproject.toml like the one below, how should it be interpreted ?

[tool.poetry.dependencies]
dependency = { version = "...", optional = true, extras = ["feature-a", "feature-b"] }

[tool.poetry.extras]
no-feature = ["dependency"]

In the current implementation, it would download both features but if we implemented the new feature like a superset that can be filtered, it would have been more logical that it download neither of the features.

That's the reason why I implemented the feature like additional extras that can be added in the tool.poetry.extras section. It is backward-compatible because the feature only applies when we add square brackets at the end of an extra and since it was not possible before, it does not break anything.

In addition, it involves only very minor modifications because we just need to add additional dependencies when we found an extra in the tool.poetry.extras section.

Finally, it also solves another problem that someone mentioned in the issue. Indeed, with this implementation, it is possible to have optional extras by writing something like this :

[tool.poetry.dependencies]
dependency = { version = "..." }

[tool.poetry.extras]
feature = ["dependency[feature]"]

With this configuration, it would install dependency[feature] only when installing my_project[feature] and it would install dependency when installing my_project.

If you are wondering why I choose to add additional extras and not to override extras, it's only because it could create inconsistencies. For example, with the following configuration, installing my_project[feature-c] would not install dependency[feature-a, feature-b] and since it is in the required section, it is not normal :

[tool.poetry.dependencies]
dependency = { version = "...", extras=["feature-a", "feature-b"] }

[tool.poetry.extras]
feature-c = ["dependency[feature-c]"]

Conclusion

Thanks for reading this far and sorry for writing so much. I really wanted to expose my ideas and it took more space than expected.

Please let me know if there are any changes that could be made, and thank you for creating this fantastic software.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[radoering]

Thank you for the comprehensive issue description. Although I'm not completely sure about the design, you have highlighted why other approaches don't work so it might be the best solution for now. (Sooner or later we'll support PEP 621 anyway, which IMO will make it easier to specify extras.)

You've already considered a lot of things I might have forgotten, but there are still a few issues.

For example poetry check is still an issue because of these lines. When running poetry check on your example pyproject.toml, you will get

Error: Cannot find dependency "transformers[torch,torch-vision]" for extra "torch" in main dependencies.

and so on. (See the inline comments for further issues.)

[dimbleby]

I'm not sure that new syntax is needed, I'd half expect something like this to work today:

foo = [
  { version = "^1.13.1" },
  { version = "^1.13.1", extras = ["their-extra", "their-other-extra"], markers = "extra == 'our-extra'" },
]

presumably it doesn't, but perhaps it wouldn't be so hard to make that work.

[radoering]

@BergLucas Can you take a look whether dimbleby's proposal is feasible or if there are any drawbacks? (This seems to be more in line with the current design.)

Further, python-poetry/poetry#6409 (comment) looks quite similar to dimbleby's proposal and your use case and it claims that it's working but only from Poetry 1.6.

[BergLucas]

@radoering It seems that dimbleby's proposal works but only if an empty list is specified for the foo extra.

[tool.poetry.extras]
foo = []

For example, if I use the following pyproject.toml with pip wheel:

[tool.poetry]
name = "extra-package"
version = "1.2.3"
description = "Some description."
authors = ["Your Name <you@example.com>"]
license = "MIT"

[tool.poetry.dependencies]
python = "^3.10"
transformers = [
    { version = "^4.30.2" },
    { version = "^4.30.2", optional = true, extras = ["tf"], markers = "extra == 'tf'"}
]

[tool.poetry.extras]
tf = []

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

It produces the following METADATA file:

Metadata-Version: 2.1
Name: extra-package
Version: 1.2.3
Summary: Some description.
License: MIT
Author: Your Name
Author-email: you@example.com
Requires-Python: >=3.10,<4.0
Classifier: License :: OSI Approved :: MIT License
Classifier: Programming Language :: Python :: 3
Classifier: Programming Language :: Python :: 3.10
Classifier: Programming Language :: Python :: 3.11
Provides-Extra: tf
Requires-Dist: transformers (>=4.30.2,<5.0.0)
Requires-Dist: transformers[tf] (>=4.30.2,<5.0.0) ; extra == "tf"

It seems that the METADATA file is correct.

I just think it's a bit confusing to specify an empty list in the extras section and then add the extras at the markers in the dependencies.

[radoering]

I just think it's a bit confusing to specify an empty list in the extras section and then add the extras at the markers in the dependencies.

We could document that it's necessary or change it so that it's not necessary. No strong feelings so far. Either way, we should probably extend the docs mentioning this alternative (more precise) method to define extras.

[dimbleby]

do poetry install --extras tf and poetry install both give the expected results in this example? I guessed that something would go wrong with this, good news if not...

[BergLucas]

You were right @dimbleby, it doesn't install the extras when I run poetry install --extras ... even if it does create the right METADATA file using pip wheel.

I tested with Poetry 1.6.1.

[dimbleby]

imo debugging and fixing that would be preferable to introducing a new syntax

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[BergLucas]

I've tried to make the changes that you two suggested and it works for poetry-core but it does not when using poetry. I don't really know why but it is weird that Poetry does not use the data from the factory correctly when installing the dependencies. Currently, it tries to install every extra instead of some of them.

For example, with the following pyproject.toml:

[tool.poetry]
name = "extra-package"
version = "1.2.3"
description = "Some description."
authors = ["Your Name <you@example.com>"]
license = "MIT"

[tool.poetry.dependencies]
python = "^3.10"
psycopg = [
    { version = "^3.1.9", optional = true , extras = ["c"], markers = "extra in 'extra-c, extra-pool'"},
    { version = "^3.1.9", optional = true , extras = ["binary"], markers = "extra == 'extra-binary'"},
    { version = "^3.1.9" },
]

[build-system]
requires = ["poetry-core@file://C:/Users/lucas/Documents/GitHub/poetry-core"]
build-backend = "poetry.core.masonry.api"

The factory produces the following data:

requires = [
    <Dependency psycopg[c] (>=3.1.9,<4.0.0)>,
    <Dependency psycopg[binary] (>=3.1.9,<4.0.0)>,
    <Dependency psycopg (>=3.1.9,<4.0.0)>
]
extras = {
    'extra-pool': [<Dependency psycopg[c] (>=3.1.9,<4.0.0)>],
    'extra-c': [<Dependency psycopg[c] (>=3.1.9,<4.0.0)>],
    'extra-binary': [<Dependency psycopg[binary] (>=3.1.9,<4.0.0)>]
}

But when I run poetry install -E extra-binary, it tries to install everything:

$ poetry install -E extra-binary

Package operations: 2 installs, 0 updates, 0 removals

  • Installing psycopg-binary (3.1.10)
  • Installing psycopg-c (3.1.10)

As for my changes, it does not work for extra != '...' at the moment and I'm not sure if I handle the MultiMarker and MultiConstraint classes correctly since it could potentially install dependencies only if multiples extras are enabled (extra == 'a' and extra == 'b').

[radoering]

Isn't that basically what dep.in_extras should be?

poetry-core/src/poetry/core/packages/dependency.py

Lines 160 to 183 in 43a2db9

	def marker(self, marker: str | BaseMarker) -> None:
	from poetry.core.constraints.version import parse_constraint
	from poetry.core.packages.utils.utils import convert_markers
	from poetry.core.version.markers import BaseMarker
	from poetry.core.version.markers import parse_marker
	if not isinstance(marker, BaseMarker):
	marker = parse_marker(marker)
	self._marker = marker
	markers = convert_markers(marker)
	if "extra" in markers:
	# If we have extras, the dependency is optional
	self.deactivate()
	for or_ in markers["extra"]:
	for op, extra in or_:
	if op == "==":
	self.in_extras.append(canonicalize_name(extra))
	elif op == "" and "||" in extra:
	for _extra in extra.split(" || "):
	self.in_extras.append(canonicalize_name(_extra))

[radoering]

How does this happen? Shouldn't extra be always be in dep.in_extras?

[radoering]

I've tried to make the changes that you two suggested and it works for poetry-core but it does not when using poetry. I don't really know why but it is weird that Poetry does not use the data from the factory correctly when installing the dependencies.

Probably, one of these methods is not sufficient for this use case.

• https://github.com/python-poetry/poetry/blob/f5b022843660988daaf4153cf3058a33b389b4f6/src/poetry/installation/installer.py#L422
• https://github.com/python-poetry/poetry/blob/f5b022843660988daaf4153cf3058a33b389b4f6/src/poetry/installation/installer.py#L374
• https://github.com/python-poetry/poetry/blob/f5b022843660988daaf4153cf3058a33b389b4f6/src/poetry/installation/installer.py#L405

[Bibo-Joshi]

Hi. Just wanted to add two more observations:

1. pip seems to be happy enough with the produced metadata. pip install . and pip install .[extra] work as expected. only tested with simple extra = '…' markers and tbh I don't quite get yet if the "extra != , and, or, in" cases are poetry specific or not :D.
2. With poetry 1.7.1, I observe the contrary of

But when I run poetry install -E extra-binary, it tries to install everything:

i.e. poetry install none of the extra requirements

[GOGKI]

Hi, will this PR be merged and closed? Is there an ETA?