Libraries that implement a client to an HTTP backend often provide multiple implementations such as for tornado, aiohttp or requests. Each implementation comes with its own dependencies. User usually needs only one implementation.
With extras_require developer of the library can specify aiohttp and torndao as optional and install requests for everyone via install_requires.
However, users of aiohttp and tornado do not need requests, hence the request to be able to specify a "default" extra that should be picked when no other extras are specified.
So that users calling pip install mylib or pip install mylib[requests] will get requests installed, but users calling pip install mylib[aiohttp] won't.
This work with pip:
from setuptools import setup
setup(name='foo', version='1.0',
install_requires='''
requests; extra == ""
''',
extras_require={
'aiohttp': 'aiohttp',
'tornado': 'tornado',
},
)
But not when setuptools' easy_install tries to install the necessary requirements (UndefinedEnvironmentName: 'extra'). For the same reason this does not work either:
from setuptools import setup
setup(name='foo', version='1.0',
extras_require={
'aiohttp': 'aiohttp',
'tornado': 'tornado',
':extra == ""': 'requests',
},
)
IMHO, both should be allowed. @jaraco: what do you think?
PEP 508 makes no mention of such behavior, so implementing this would cause setuptools to fall out of compliance, IMHO.
Perhaps environment should provide extra == '' when no extra is specified. That would make the first example by @benoit-pierre to work.
@agronholm PEP 508 specifies the "extra" marker but it doesn't seem to specify its value when it's not set.
I've just been experimenting with this problem...
This has broken for me since 36.2. Since then, if you use extra inside your install_requires you hit the problem @benoit-pierre describes above. Stack for me looks like this:
Traceback (most recent call last):
File "setup.py", line 3, in <module>
setup(name='foo', zip_safe=False, )
...
File "/home/peter/setuptools/pkg_resources/__init__.py", line 2661, in _dep_map
if invalid_marker(marker):
File "/home/peter/setuptools/pkg_resources/__init__.py", line 1441, in invalid_marker
evaluate_marker(text)
File "/home/peter/setuptools/pkg_resources/__init__.py", line 1459, in evaluate_marker
return marker.evaluate()
File "/home/peter/setuptools/pkg_resources/_vendor/packaging/markers.py", line 301, in evaluate
return _evaluate_markers(self._markers, current_environment)
File "/home/peter/setuptools/pkg_resources/_vendor/packaging/markers.py", line 226, in _evaluate_markers
lhs_value = _get_env(environment, lhs.value)
File "/home/peter/setuptools/pkg_resources/_vendor/packaging/markers.py", line 208, in _get_env
"{0!r} does not exist in evaluation environment.".format(name)
pkg_resources._vendor.packaging.markers.UndefinedEnvironmentName: 'extra' does not exist in evaluation environment.
However, invalid_marker is just trying to check if a marker is valid or not, so I wondered about catching the exception there and just saying it's not valid (by returning the exception rather than False). So I created this patch.
diff --git a/pkg_resources/__init__.py b/pkg_resources/__init__.py
index 7333464..71f614e 100644
--- a/pkg_resources/__init__.py
+++ b/pkg_resources/__init__.py
@@ -1439,6 +1439,8 @@ def invalid_marker(text):
"""
try:
evaluate_marker(text)
+ except packaging.markers.UndefinedEnvironmentName as e:
+ return e
except SyntaxError as e:
e.filename = None
e.lineno = None
diff --git a/setuptools/tests/test_egg_info.py b/setuptools/tests/test_egg_info.py
index 66ca916..b2cd384 100644
--- a/setuptools/tests/test_egg_info.py
+++ b/setuptools/tests/test_egg_info.py
@@ -284,6 +284,19 @@ class TestEggInfo(object):
''',
'''
+ install_requires_with_extra_marker
+
+ install_requires=["barbazquux; extra != 'foo'"],
+
+ [options]
+ install_requires =
+ barbazquux; extra != 'foo'
+
+ [:extra != "foo"]
+ barbazquux
+ ''',
+
+ '''
Seems to do the trick for me! Would you like me to submit a PR?
I don't think this is right. A better alternative would be to always define the extra environment marker, setting it to None if it's not specified. I think that's what pip does.
Yeah - that would work too. I'm not sure if that aligns with pep 508, though.
The "extra" variable is special. It is used by wheels to signal which specifications apply to a given extra in the wheel METADATA file, but since the METADATA file is based on a draft version of PEP-426, there is no current specification for this. Regardless, outside of a context where this special handling is taking place, the "extra" variable should result in an error like all other unknown variables.
Yeah... I don't know why it was specified as such. It makes some things really hard, like converting metadata from wheel to egg: https://github.com/pypa/setuptools/blob/master/setuptools/wheel.py#L105
Lovely! :-)
I'm still new to the setuptools code, so don't know where the extra variable would need to be defined in a safe way. Happy to have a look at that for a PR instead if that's what's needed, but a pointer would be appreciated...
I think this is what I had:
diff --git a/pkg_resources/__init__.py b/pkg_resources/__init__.py
index 6f1071fb..db241642 100644
--- a/pkg_resources/__init__.py
+++ b/pkg_resources/__init__.py
@@ -1456,7 +1456,7 @@ def evaluate_marker(text, extra=None):
"""
try:
marker = packaging.markers.Marker(text)
- return marker.evaluate()
+ return marker.evaluate({'extra': extra})
except packaging.markers.InvalidMarker as e:
raise SyntaxError(e)
@@ -2678,7 +2678,7 @@ class Distribution(object):
if invalid_marker(marker):
# XXX warn
reqs = []
- elif not evaluate_marker(marker):
+ elif not evaluate_marker(marker, extra=extra):
reqs = []
extra = safe_extra(extra) or None
dm.setdefault(extra, []).extend(parse_requirements(reqs))
Looks like that's not quite enough... I get this error when I run my test input (from the previous diff) against it.
E AssertionError: warning: install_lib: 'build/lib' does not exist -- no Python modules to install
E
E Couldn't find index page for 'barbazquux' (maybe misspelled?)
E No local packages or working download links found for barbazquux
E error: Could not find suitable distribution for Requirement.parse('barbazquux')
That's normal, no? Your test is trying to install a requirement that cannot be met, change it to:
setuptools/tests/test_egg_info.py | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git i/setuptools/tests/test_egg_info.py w/setuptools/tests/test_egg_info.py
index 66ca9164..b270ba25 100644
--- i/setuptools/tests/test_egg_info.py
+++ w/setuptools/tests/test_egg_info.py
@@ -373,6 +373,20 @@ class TestEggInfo(object):
[empty]
''',
+
+ '''
+ install_requires_with_extra_marker
+
+ install_requires=["pytz; extra != 'foo'"],
+
+ [options]
+ install_requires =
+ pytz; extra != 'foo'
+
+ [:extra != "foo"]
+ pytz
+ ''',
+
# Format arguments.
invalid_marker=invalid_marker,
mismatch_marker=mismatch_marker,
Sorry - I was being dense there. I'm clearly trying to do too much in too little time and making careless mistakes as a result. Is it worth packaging this fix up as a PR?
No, this does not work... Because of the way the dependency map is built when using a dist-info distribution (wheel install), pytz will be added to the common requirements (and installed no matter the extra).
@benoit-pierre Still finding my way around this code... I think you're talking about invoking the Wheel.install_as_egg() path. When I try setting up a test to invoke that, the resulting PKG-INFO file contains:
Requires-Dist: foobar; extra == "bar"
And the metedata.json also has conditional content:
{"description_content_type": "UNKNOWN", "extensions": {"python.details": {"document_names": {"description": "DESCRIPTION.rst"}}}, "extras": [], "generator": "bdist_wheel (0.30.0)", "metadata_version": "2.0", "name": "foo", "run_requires": [{"environment": "extra == \"bar\"", "requires": ["foobar"]}], "summary": "UNKNOWN", "version": "1.0"}
That all looks plausible to me... What have I missed?
Bump @benoit-pierre! Would love to see this pushed forward in some way if you have time.
Would using the empty-string extra as a default be a reasonable approach here?
from setuptools import setup
setup(name='foo', version='1.0',
extras_require={
'aiohttp': 'aiohttp',
'tornado': 'tornado',
'': 'requests',
},
)
I was able to get this to work via setuptools with a two-line change to pkg_resources:
diff --git a/pkg_resources/__init__.py b/pkg_resources/__init__.py
index 3ae2c5cd..2226b8ae 100644
--- a/pkg_resources/__init__.py
+++ b/pkg_resources/__init__.py
@@ -2628,6 +2628,8 @@ class Distribution:
raise UnknownExtra(
"%s has no such extra feature %r" % (self, ext)
)
+ if not extras:
+ deps.extend(dm.get('', ()))
return deps
def _get_metadata(self, name):
Presumably the empty-string-extra is not used elsewhere, and running pip install foo[] already succeeds and installs all the same dependencies as pip install foo for the package as one would expect.
~I made a PR: https://github.com/pypa/setuptools/pull/1503~ Not going to work given the current state of things.
The option to have one extra the default one really a good idea.
In the new Asyncio world many useful libraries are replaced by completely different ones.
Any update on this? This could actually be really useful!
Any update ?
CC: @benoit-pierre
I want to give this a strong +1 as useful feature. I have different use case.
I currently have package which provides an ABC and a number of implementation each which have there own dependencies. I now want to break out each implementation into its own extra:
# Old
setup(
...
instal_requires=['A', 'B', 'C'],
)
# New
setup(
...
instal_requires=[],
extra_reqires = dict(A=['A'], B=['B'], C=['C'], ALL=['A', 'B', 'C']),
)
But this would break any downstream users install scripts if they expect package[ALL].
At the same time I have users that need a "minimal install". This would not be a problem
if I could default require ALL.
I want to give this a strong +1 as useful feature. I have different use case.
This is exactly the problem that we had in gql (See issue gql #147).
We finally decided to implement a breaking change in order to have optional dependencies.
Most helpful comment
The option to have one extra the default one really a good idea.
In the new Asyncio world many useful libraries are replaced by completely different ones.