AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
960 stars 338 forks source link

rez pip package with dot in name, but written with dash in requirements #1545

Open lpequignot opened 1 year ago

lpequignot commented 1 year ago

Hi,

We are using prefect package https://github.com/PrefectHQ/prefect and we are using rez and rez pip to install it. One of it's dependencies is ruamel.yaml, package with a dot in the name. The issue is that it creates a ruamel.yaml rez package on disk but it's adding "ruamel_yaml-0.17.0+" in prefect package.py requires as follow.


name = 'prefect'

version = '2.13.6'

description = 'Workflow orchestration and management.'

authors = ['Prefect Technologies, Inc. help@prefect.io']

tools = ['prefect']

requires = [
    'fsspec-2022.5.0+',
    'starlette-0.27.0+<0.28.0',
    'graphviz-0.20.1+',
    'jsonpatch-1.32+<2.0',
    'PyYAML-5.4.1+<7.0.0',
    'alembic-1.7.5+<2.0.0',
    'pydantic-1.10.0+<2.0.0',
    'rich-11.0+<14.0',
    'click-8.0+<8.2',
    'python_slugify-5.0+<9.0',
    'docker-4.0+<7.0',
    'cloudpickle-2.0+<3.0',
    'websockets-10.4+',
    'coolname-1.0.4+<3.0.0',
    'Jinja2-3.0.0+<4.0.0',
    'aiosqlite-0.17.0+',
    'anyio-3.7.1+<4.0.0',
    'cryptography-36.0.1+',
    'kubernetes-24.2.0+<29.0.0',
    'dateparser-1.1.1+<2.0.0',
    'griffe-0.20.0+',
    'apprise-1.1.0+<2.0.0',
    'pathspec-0.8.0+',
    'SQLAlchemy-1.4.22+<1.4.33|1.4.33.1+<3.0.0',
    'asgi_lifespan-1.0+<3.0',
    'croniter-1.0.12+<2.0.0',
    'uvicorn-0.14.0+',
    'orjson-3.7+<4.0',
    'typer-0.4.2+',
    'packaging-21.3+<24.3',
    'typing_extensions-4.5.0+<5.0.0',
    'asyncpg-0.23+',
    'httpx-0.23+<0.23.2|0.23.2.1+',
    'pytz-2021.1+<2024',
    'toml-0.10.0+',
    'readchar-4.0.0+<5.0.0',
    'pendulum-2.1.2+<3.0.0',
    **'ruamel_yaml-0.17.0+',**
    'jsonschema-3.2.0+<5.0.0'
]

variants = [['platform-linux', 'arch-x86_64', 'python-3.9', 'importlib_metadata-4.4+']]

def commands():
    env.PYTHONPATH.append('{root}/python')
    env.PATH.append('{root}/bin')

help = [['Home Page', 'https://www.prefect.io']]

timestamp = 1697205164

hashed_variants = True

pip_name = 'prefect (2.13.6)'

from_pip = True

is_pure_python = True

format_version = 2

So, once we want to rez-env prefect it fails with the following error. 12:15:03 ERROR PackageFamilyNotFoundError: package family not found: ruamel_yaml, was required by: prefect

It seems to be due to the fact that in prefect requirements it's written ruamel-yaml >= 0.17.0 https://github.com/PrefectHQ/prefect/blob/main/requirements.txt#L34

And in rez pip_to_rez_package_name it's converting "-" to "_" https://github.com/AcademySoftwareFoundation/rez/blob/main/src/rez/utils/pip.py#L27

Do you have a suggestion on how we could solve this (in our rez config , or with a hook maybe) ?

Regards,

Louise

Environment

To Reproduce

  1. rez pip -i prefect

Expected behavior package name in requires is the same as the rez package installed,

Actual behavior It creates a ruamel.yaml rez package on disk but it's adding "ruamel_yaml-0.17.0+" in prefect package.py requires. rez-env prefect fails with 12:15:03 ERROR PackageFamilyNotFoundError: package family not found: ruamel_yaml, was required by: prefect

Related Issues/PRs

JeanChristopheMorinPerso commented 1 year ago

I'm wondering if it has to do with https://github.com/AcademySoftwareFoundation/rez/blob/main/src/rez/utils/pip.py#L378-L385.

lpequignot commented 1 year ago

Yes, the error comes from prefect rather than rez as prefect requirements use ruamel-yaml >= 0.17.0 instead of ruamel.yaml >= 0.17.0 Maybe I can ask if it's possible to change it.

JeanChristopheMorinPerso commented 1 year ago

You can definitely request to have it changed, or even better, open a PR :) But it doesn't change the fact that we have a bug, and a rather serious one IMO.

As for my last reply, my comment was about the fact that we do a mapping between uppercased and lowercased packages. So maybe we should do something like this but for - and .... But I have no idea how we should do it and how safe it is to do...

JeanChristopheMorinPerso commented 1 year ago

Ok, I took a deeper look and I think I understand what's going on. https://github.com/AcademySoftwareFoundation/rez/blob/main/src/rez/utils/pip.py#L42 only replaces - with _. If we were to also replace . with _, I think it would fix the issue. So my initial guess was approximately right.

Fixing this would probably be kind of a breaking change though. I will definitely fix it in https://github.com/JeanChristopheMorinPerso/rez-pip, but I have no plans to fix it in rez itself for now.

JeanChristopheMorinPerso commented 1 year ago

https://github.com/JeanChristopheMorinPerso/rez-pip/issues/55

lpequignot commented 1 year ago

Hi,

For the follow up, I've created a PR for prefect requirements https://github.com/PrefectHQ/prefect/pull/10987 It has been approved and merged this morning.

Thank you