ohsu-comp-bio / py-tes

Python SDK for the GA4GH Task Execution API
https://py-tes.readthedocs.io/
MIT License
5 stars 10 forks source link

`python-dateutil` requirement in setup.py more specific than in `requirements.txt` #34

Closed adamnovak closed 3 years ago

adamnovak commented 3 years ago

setup.py requires exactly python-dateutil 2.6.1, while requirements.txt asks for that version or later.

https://github.com/ohsu-comp-bio/py-tes/blob/7606dc8c74fb69ac65e1fb3acf8c16aa4cb42d9e/setup.py#L44

https://github.com/ohsu-comp-bio/py-tes/blob/7606dc8c74fb69ac65e1fb3acf8c16aa4cb42d9e/requirements.txt#L3

This is managing to confuse pip in my usage (possibly with the old resolver that can't intersect requirements). I end up with some of my dependent package entrypoints bailing out with a pkg_resources.ContextualVersionConflict; my package has its own dependency on just python-dateutil at any version.

Traceback (most recent call last):
  File "/builds/databiosphere/toil/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 568, in _build_master
    ws.require(__requires__)
  File "/builds/databiosphere/toil/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 886, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/builds/databiosphere/toil/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 777, in resolve
    raise VersionConflict(dist, req).with_context(dependent_req)
pkg_resources.ContextualVersionConflict: (python-dateutil 2.8.2 (/builds/databiosphere/toil/venv/lib/python3.8/site-packages), Requirement.parse('python-dateutil==2.6.1'), {'py-tes'})

The setup.py requirement should be changed to specify a minimum version, like the requirements.txt does. If a <3 is needed, that could be added, but I don't think there's a good reason to pin this exact version. What if someone wants to use py-tes in a package that needs a new python-dateutil feature, or that also depends on another module that does this same thing but with a different version of python-dateutil?

kellrott commented 3 years ago

Does #35 fix the issue?

adamnovak commented 3 years ago

I pulled it in for testing in https://ucsc-ci.com/databiosphere/toil/-/jobs/97312. If not I would expect to see those messages recur there.

adamnovak commented 3 years ago

OK, it looks like the fix in #35 does in fact work!