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

Add support for environment markers in dependency specifiers #4813

Closed
agriyakhetarpal opened this issue May 23, 2024 · 4 comments · Fixed by #4812
Closed

Add support for environment markers in dependency specifiers #4813

agriyakhetarpal opened this issue May 23, 2024 · 4 comments · Fixed by #4812
Labels
bug Something isn't working

Comments

@agriyakhetarpal
Copy link
Member

agriyakhetarpal commented May 23, 2024

Description

micropip at the time of writing apparently does not work with dependency specifiers – specifically, environment markers.

A reproducer is as follows:

pyproject.toml
[project]
name = "mypackage"
dependencies = [
    'asciitree',
    'numpy>=1.26.2',  # this works
    'fasteners; sys_platform != "emscripten"',  # or 'fasteners; platform_system != "Emscripten"'
    'scipy',
    # ...
]
license = { text = "BSD-3-Clause" }
# and so on

which ignores the restriction I put in place on fasteners, and proceeds to install it anyway (workflow logs). Though, I understand that this example might be a stub – another edge case with some additional context plus more logs is available below.

Expected behaviour

The expected behaviour would be to support the parsing of environment-marker-based dependency specifiers when installing a package and therefore conditionally include or exclude dependencies from PEP-621-style dependency metadata in pyproject.toml through the supported grammar mentioned in the Python Packaging guide.

In this case, it should exclude fasteners.

Additional context

I stumbled across this when I was trying to add out-of-tree Pyodide builds for the Zarr package (xref: zarr-developers/zarr-python#1903), where zarr requires numcodecs as a dependency, but it does not include the [msgpack] optional dependency for numcodecs. This makes the tests suite fail to run because of import errors across the package (not sure about the reason) – which is why I was trying to add something like this:

dependencies = [
    'numcodecs>=0.10.0',
    'numcodecs[msgpack]>=0.10.0; platform_system == "Emscripten"',

to conditionally include the msgpack dependency when running in a Pyodide virtual environment, but this does not currently work and micropip does not install msgpack (the [msgpack] optional dependency is defined to install just the msgpack package as of now).

However, if I remove platform_system == "Emscripten" from the above TOML table, and keep just numcodecs[msgpack]>=0.10.0, it does install msgpack as expected (workflow logs).

While I can add msgpack as a dependency directly at this time because there is no pin on its version, it could cause issues in situations where there would be constraints on optional dependencies (perhaps pyodide/micropip#93 would be helpful there) – say, if numcodecs[msgpack] were to require msgpack>=0.12.0, I would have to install msgpack>=0.12.0 in a further and separate pip install ... command invocation, which is probably okay for one dependency or two, but for cases where there can be several optional dependencies, there would not be much of a choice but to either carefully (and manually) sync in multiple locations the versions of the packages being installed, or prepare a lockfile beforehand and keep updating it.

@ryanking13 ryanking13 added the bug Something isn't working label May 24, 2024
@ryanking13
Copy link
Member

Thanks for the report! This looks like a bug... but I am not sure this is a bug in micropip. @hoodmane does pyodide venv rely on micropip when installing packages? I guess platform_system is somehow incorrect in pyodide venv.

@ryanking13
Copy link
Member

One thing I suspect is that in the browser, I get the following values for packaging.markers.default_environment().

  • 'platform_system': 'Emscripten' (platform.system)
  • 'sys_platform': 'emscripten' (sys.platform)

While in pyodide venv, both platform.system and sys.platform seem to be set to emscripten (note that the first letter is not capital) (code pointer)

@agriyakhetarpal
Copy link
Member Author

Thanks for the report! This looks like a bug... but I am not sure this is a bug in micropip.

Oops, thanks! Please feel free to transfer this issue over to the Pyodide repository if that would make better sense.

Also, thanks for the pointer – I'll probably need to read a bit more to investigate, but considering that we need to manage and ensure the return of just two values ("Emscripten" versus "emscripten"), that should be an easier fix for now.

@hoodmane
Copy link
Member

I think this is a tiny fix, I'll take care of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants