conda / conda-lock

Lightweight lockfile for conda environments
https://conda.github.io/conda-lock/
Other
456 stars 101 forks source link

Fix case when using multiple platforms but pip contains a single platform #564

Closed basnijholt closed 7 months ago

basnijholt commented 7 months ago

This came up in https://github.com/basnijholt/unidep/pull/23. This UniDep is a package that unifies Conda and Pip dependency specification and integrates with conda-lock. I think conda-lock folks might like it :smile:

About this fix, it prevents:

  File "/Users/bas.nijholt/micromamba/lib/python3.11/site-packages/conda_lock/src_parser/environment_yaml.py", line 58, in _parse_environment_file_for_platform
    for spec in mapping_spec["pip"]:
TypeError: 'NoneType' object is not iterable

Which occurs when locking:

name: example
channels:
  - conda-forge
dependencies:
  - tomli
  - pip:
    - psutil  # [linux64]
platforms:
  - linux-64
  - osx-arm64

Which becomes:

env_yaml_data = {'name': 'test-pip-with-platform-selector', 'channels': ['conda-forge'], 'dependencies': ['tomli', {'pip': None}], 'platforms': ['linux-64', 'osx-arm64']}

For osx-arm64.

This fix skips {"pip": None}.

netlify[bot] commented 7 months ago

Deploy Preview for conda-lock ready!

Name Link
Latest commit 6ee68f168e8e067f4968afbcb980aea6d1d58fde
Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/656a033e8e990c0008d2b5da
Deploy Preview https://deploy-preview-564--conda-lock.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

maresb commented 7 months ago

Ah, took me a bit to figure out exactly what this is about. Here's my summary:

For the osx-arm64 platform, after filtering

  - pip:
    - psutil  # [linux64]

becomes

  - pip:

then we parse this like

yaml.safe_load("pip:")  # {'pip': None}

so we must deal with the edge case mapping_spec["pip"] = None and not just mapping_spec["pip"] = [].

This looks great! Only thing is that this introduces an orphaned environment.yml in the tests directory. Would you be able to write a test to check that your environment.yml parses correctly and that psutil is present only for linux64?


And wow, UniDep looks awesome!!! It would be great if we could collaborate. Apart from getting fresh eyes on the codebase to clean up stuff like this, I really wish that conda-lock could output more intermediate representations along the lines of your requirements.yaml.

maresb commented 7 months ago

Thanks @basnijholt!!!