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

Poetry refuses to upgrade projects that bound the Python version #4292

Closed
NeilGirdhar opened this issue Jul 16, 2021 · 13 comments
Closed

Poetry refuses to upgrade projects that bound the Python version #4292

NeilGirdhar opened this issue Jul 16, 2021 · 13 comments
Labels
kind/bug Something isn't working as expected

Comments

@NeilGirdhar
Copy link
Contributor

Please see my comment and @rgommers's reply here.

Because SciPy (and soon NumPy) have an upper bound version set to be "<3.10", Poetry refuses to upgrade SciPy on any project that leaves the Python version unbounded. Poetry says:

   1: fact: scipy (1.7.0) requires Python >=3.7,<3.10
   1: derived: not scipy (==1.7.0)
   1: fact: scipy (1.6.3) requires Python >=3.7,<3.10
   1: derived: not scipy (==1.6.3)
   1: fact: scipy (1.6.2) requires Python >=3.7,<3.10
   1: derived: not scipy (==1.6.2)
   1: fact: scipy (1.6.0) depends on numpy (>=1.16.5)
   1: selecting scipy (1.6.0)

This means that all projects either need to inherit this upper bound in order to get updates. Does that make sense? Or would it be better for Poetry to not derive the above facts? After all, when poetry update is called, the Python version is fixed.

@NeilGirdhar NeilGirdhar added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Jul 16, 2021
@rgommers
Copy link

@NeilGirdhar it's still unclear to me how/why Poetry wants to use Python 3.10 here. Or where it would even get it from. Can you add the relevant part of your config?

@NeilGirdhar
Copy link
Contributor Author

NeilGirdhar commented Jul 16, 2021

@rgommers It's not that Poetry "wants to use 3.10". I'm not an expert, but I'll do my best to explain.

Suppose you have a project with pyproject.toml that has:

[tool.poetry.dependencies]
python = ">=3.7.1"
scipy = ">=1.5"

When poetry update is run, it generates a lock file with versions that are shared between all developers of your project. This lock file is used to install libraries into the development virtual environment.

When choosing the versions for the development environment, Poetry will reason that since your project could be installed on Python 3.10, and SciPy 1.7 is not compatible with 3.10, then Poetry should not choose SciPy 1.7 for the developer lock file.

Of course, if you install your project using pip, you will still get SciPy 1.7. This issue is only about the development environment.

@rgommers
Copy link

Thanks, makes sense now.

When choosing the versions for the development environment, Poetry will reason that since your project could be installed on Python 3.10, and SciPy 1.7 is not compatible with 3.10, then Poetry should not choose SciPy 1.7 for the developer lock file.

I'm not sure if I'd consider it user error or a Poetry error. The >=3.7.1 meaning "this project is compatible with any future Python version ever" will of course be false for any nontrivial project (so user error). But this is not a lock file shipped in a release, this is for specifying a dev environment. Normally you leave dependencies without an upper bound in your main branch, and you add upper bounds (or sometimes exact pins) for package releases.

I still don't get what the output is here. Why does it need to choose a fixed SciPy version, but is happy to not fix the Python version? If you want to lock exact versions as of today, the correct output is something like python == 3.9, scipy == 1.7.0.

@NeilGirdhar
Copy link
Contributor Author

NeilGirdhar commented Jul 16, 2021

Yes, it could be user error. Perhaps the Poetry people can advise me 😄.

As for your idea of specifying a different Python dependence for devevelopment and release, as far as I know, Poetry doesn't have this option.

I still don't get what the output is here. Why does it need to choose a fixed SciPy version, but is happy to not fix the Python version?

The Python version is also fixed in your development environment. If you haven't played with poetry yet, feel free to try it out:

pip install poetry
git clone git@github.com:NeilGirdhar/tjax.git
cd tjax
poetry install

This will recreate my development environment on your machine.

You can inspect the update process with:

poetry update -vv | less

@rgommers
Copy link

If you haven't played with poetry yet, feel free to try it out:

Thanks for the example. I played with Poetry a little, and recommend it to people who prefer PyPI over conda-forge, but not enough to understand this type of dev workflow use case.

@rgommers
Copy link

It indeed refuses to upgrade scipy. In your pyproject.toml, scipy is only a transitive dependency of jaxlib. If I add an explicit scipy = "==1.7.0" as a dependency and then run poetry update -vv, the error is clear and there's even a correct valid solution suggested at the end:

  SolverProblemError

  The current project's Python requirement (>=3.7.1,<4) is not compatible with some of the required packages Python requirement:
    - scipy requires Python >=3.7,<3.10, so it will not be satisfied for Python >=3.10,<4
  
  Because tjax depends on scipy (1.7.0) which requires Python >=3.7,<3.10, version solving failed.

  at ~/anaconda3/envs/poetry/lib/python3.9/site-packages/poetry/puzzle/solver.py:241 in _solve
      237│             packages = result.packages
      238│         except OverrideNeeded as e:
      239│             return self.solve_in_compatibility_mode(e.overrides, use_latest=use_latest)
      240│         except SolveFailure as e:
    → 241│             raise SolverProblemError(e)
      242│ 
      243│         results = dict(
      244│             depth_first_search(
      245│                 PackageNode(self._package, packages), aggregate_package_nodes

  • Check your dependencies Python requirement: The Python requirement can be specified via the `python` or `markers` properties
    
    For scipy, a possible solution would be to set the `python` property to ">=3.7.1,<3.10"

    https://python-poetry.org/docs/dependency-specification/#python-restricted-dependencies,
    https://python-poetry.org/docs/dependency-specification/#using-environment-markers

The diagnostic output from Poetry is impressively good I'd say. The proposed resolution seems fine to me.

@NeilGirdhar
Copy link
Contributor Author

@rgommers Okay, so I should inherit your bound of <3.10? I wasn't sure, and thought that that would be unnecessary work in bumping that bound on all my projects every time a new Python version comes out. I guess that's what I should do then.

@rgommers
Copy link

I wasn't sure, and thought that that would be unnecessary work in bumping that bound on all my projects every time a new Python version comes out

It'd be nice if it wasn't needed, I think it's still odd that it is - but that's up to the Poetry devs.

@hwalinga
Copy link

hwalinga commented Aug 4, 2021

@rgommers

I just wanted to add that this is not entirely a Poetry problem. Pip will do the same.

So, I was trying out 3.10 and if I run pip install scipy, pip will happily pick 1.6.1 for me to install on 3.10. It will even install on 3.10. But of course is riddled with problems that don't work on 3.10. Didn't found any problems with manually overriding pip with --ignore-requires-python and testing out 1.7.1.

So, I understand the rationale, but some eager people for 3.10 will get bitten by this newly introduced bound, and I guess it probably won't be able to retroactively apply this to old versions of scipy on pypi.

@rgommers
Copy link

rgommers commented Aug 6, 2021

@hwalinga yes, this is a known problem with Pip. The current behavior makes very little sense. Defaulting to --only-binary if a package has wheels looks like a decent solution direction, see pypa/pip#9140. It'd be good if Poetry did this as well.

So, I understand the rationale, but some eager people for 3.10 will get bitten by this newly introduced bound, and I guess it probably won't be able to retroactively apply this to old versions of scipy on pypi.

We may be able to add wheels before 3.10.0 is released, but it's extra work and we take a little gamble then that CPython c API/ABI doesn't change anymore.

If package installers don't fix this problem, the other option is at some point we'll upload a single sdist-only release which immediately errors out for all versions of Python. E.g. release a normal 1.8.1 with sdist + wheels, and then immediately after a "circuit breaker" sdist with a 1.8.0 version. That may sound like a heavy-handed approach, but imho it isn't - packages like TensorFlow and PyTorch upload no sdists at all exactly because of this issue, which would be a much worse solution for everyone (sdists kind of belong on PyPI).

@NeilGirdhar
Copy link
Contributor Author

NeilGirdhar commented Aug 6, 2021

It'd be good if Poetry did this as well.

Perhaps you should open an issue? The poetry people have been very responsive. Normally, I would open the issue, but I'm completely out of my depth here, and you appear to be an expert 😄

If I understand correctly, in poetry's model, you want poetry to "derive facts" that only versions with wheels are considered if there are any wheels at all?

Also, I'm just curious, but do packages ever go from releasing wheels to not releasing wheels?--in which case your idea would make it so that you would be stuck using past versions?

@rgommers
Copy link

rgommers commented Aug 6, 2021

Also, I'm just curious, but do packages ever go from releasing wheels to not releasing wheels?--in which case your idea would make it so that you would be stuck using past versions?

Highly unusual. In most cases it's a temporary issue, like the release manager uploading an sdist first and the wheels after. I've done this before and gotten bug reports about it within an hour, it's an easy mistake to make.

There's a way to accommodate those projects though that do it on purpose, there's a design tradeoff to make here between:

  1. don't accept sdists at all if a project has a wheel for any version
  2. accept sdists only if there are no wheels for that version or any newer version

(2) seems good enough and would not break anything for projects that at some point decided to stop shipping wheels.

Perhaps you should open an issue?

I will try to do that sometime in the next week, with some more context and a suggested design change.

Copy link

github-actions bot commented Mar 2, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

4 participants