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

Fix poetry python version limits #16

Closed
wants to merge 1 commit into from
Closed

Fix poetry python version limits #16

wants to merge 1 commit into from

Conversation

mattsta
Copy link

@mattsta mattsta commented Jul 28, 2022

The grpc dependency doesn't build with Python 3.10 yet because it relies
on outdated setuptools which the dependency tree isn't managing
automatically, so don't allow environments under 3.10+ until
grpc gets their act together.

Commit also updates minor versions of some deps because re-synchronizing
poetry.lock with pyproject requires a full "poetry update"
which pulls updated minor dependency versions everywhere.

The grpc dependency doesn't build with Python 3.10 yet because it relies
on outdated setuptools which the dependency tree isn't managing
automatically, so don't allow environments under 3.10+ until
grpc gets their act together.

Commit also updates minor versions of some deps because re-synchronizing
poetry.lock with pyproject requires a full "poetry update"
which pulls updated minor dependency versions everywhere.
@krasserm
Copy link
Owner

Thanks for your pull request @mattsta. What error do you see? I just updated the Python version in environment.yml to 3.10, created a new conda env, installed from sources (current main) and via pip and it just works fine. What am I missing?

@mattsta
Copy link
Author

mattsta commented Jul 29, 2022

oh, i was going off two parts: the CI scripts are all pinned to 3.9 and it didn't install under 3.10 for me, so figured 3.10 was off limits for now.

shortcircuting my debugging and jumping to the answer: looks like a bug in poetry only fixed on pre-release versions at the moment (pip install poetry --pre -U) — after that it works again: python-poetry/poetry#4242

so with poetry, current install works under python3.9 and poetry latest release; but for python3.10 it needs poetry pre-release. and it apparently works under any situation with conda because maybe conda isn't allowing poetry to uninstall setuptools in the venv?

original logic

with a current release poetry and running poetry install (no conda, no pip) under python 3.10, it actually uninstalls setuptools first for some reason which is probably what's breaking grpc (dependency of tensorboard) downstream (but it doesn't do that under python 3.9):

$ poetry install
Creating virtualenv perceiver-io-_lGgSPQn-py3.10 in /Users/matt/Library/Caches/pypoetry/virtualenvs
Installing dependencies from lock file

Package operations: 71 installs, 0 updates, 2 removals

   Removing setuptools (63.2.0)
   Removing wheel (0.37.1)
.
.
.
   Installing grpcio (1.47.0): Failed

  EnvCommandError

  Command ['/Users/matt/Library/Caches/pypoetry/virtualenvs/perceiver-io-_lGgSPQn-py3.10/bin/pip', 'install', '--no-deps', 'file:///Users/matt/Library/Caches/pypoetry/artifacts/c3/72/01/f75df338c4808aa2d963201698938a007025dcccbb3455fff481e5b9d6/grpcio-1.47.0.tar.gz'] errored with the following return code 1, and output: 
  Processing /Users/matt/Library/Caches/pypoetry/artifacts/c3/72/01/f75df338c4808aa2d963201698938a007025dcccbb3455fff481e5b9d6/grpcio-1.47.0.tar.gz
    Preparing metadata (setup.py): started
    Preparing metadata (setup.py): finished with status 'error'
    ERROR: Command errored out with exit status 1:
     command: /Users/matt/Library/Caches/pypoetry/virtualenvs/perceiver-io-_lGgSPQn-py3.10/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/var/folders/sk/zncv9ddn375grsn9fddr0hj00000gn/T/pip-req-build-1bhoqi1v/setup.py'"'"'; __file__='"'"'/private/var/folders/sk/zncv9ddn375grsn9fddr0hj00000gn/T/pip-req-build-1bhoqi1v/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /private/var/folders/sk/zncv9ddn375grsn9fddr0hj00000gn/T/pip-pip-egg-info-3p63hpo4
         cwd: /private/var/folders/sk/zncv9ddn375grsn9fddr0hj00000gn/T/pip-req-build-1bhoqi1v/
    Complete output (3 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    ModuleNotFoundError: No module named 'setuptools'
    ----------------------------------------
  WARNING: Discarding file:///Users/matt/Library/Caches/pypoetry/artifacts/c3/72/01/f75df338c4808aa2d963201698938a007025dcccbb3455fff481e5b9d6/grpcio-1.47.0.tar.gz. Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
  ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
  WARNING: You are using pip version 21.3.1; however, version 22.2.1 is available.
  You should consider upgrading via the '/Users/matt/Library/Caches/pypoetry/virtualenvs/perceiver-io-_lGgSPQn-py3.10/bin/python -m pip install --upgrade pip' command.

also the current poetry version breaks poetry update with this package configuration under python3.10 too (it got stuck in an infinite loop I canceled after 25 minutes and 20 GB RAM of poetry growth) 🤷‍♂️

thanks for verifying though! it was a combination of poetry holding back bugfixes for 3+ months until they decide to bundle a new official release plus python versions doing weird things against existing poetry versions.

it looks like somehow you were also aware of poetry doing weird things with setuptools (but it doesn't break the install under 3.9) because CI is manually re-adding it here:

- name: Install dependencies
run: |
poetry install
poetry run pip install setuptools==59.5.0
poetry run python --version
poetry run pip --version
poetry run pip list

@cstub
Copy link
Collaborator

cstub commented Jul 30, 2022

Hi @mattsta. First of all, thanks for the contribution.
The minimal change necessary to fix this issue is to set the python version to <3.10 in the pyproject.toml file (as you did) and call poetry lock --no-update to sync the .lock-file without updating existing dependencies.

I am not sure that we should proceed with this change though, because this would make the project incompatible with python 3.10 which is not the case if the installation instructions are followed and a recent enough poetry version is used. Currently, this requires a beta-version of poetry (which is admittedly not ideal) but once the 1.2-release of poetry is out this should not be issue.

I would rather keep the current state where the project can be used in a python 3.10 environment.

@mattsta
Copy link
Author

mattsta commented Jul 30, 2022

Yup, exactly right.

My original assumption the child dependencies were un-buildable under 3.10+ was wrong, so the proper fix is just a small process improvement.

Maybe just a doc note (and maybe add python 3.10 and 3.11 to the CI matrix to confirm it stays working?) saying this project needs poetry pre-release under 3.10+ (since poetry 1.2 has been in alpha/beta mode for over a year now and they keep pushing back the release date 🤷‍♂️), so just a quick FYI about poetry currently stuck in a weird state should be enough for everybody to keep it working across versions.

[edit: though, now i see it clearly already said 1.2.0b2 or higher in the readme! bah, didn't even notice because i figured the latest pip version was up to date enough instead of being a secret pre-release version. oh well. i'm just a simple person who blindly does poetry install when i see a pyproject with lockfile nearby 🫠]

Other build systems like CMake actually let us say "only CMake version x.y.z+ is allowed to build this project due to feature/compat changes over time," but looks like poetry doesn't allow specifying a minimum poetry version for building:

Thanks for the feedback and runaround on a partial non-issue!

@mattsta mattsta closed this Jul 31, 2022
@cstub
Copy link
Collaborator

cstub commented Aug 1, 2022

Thanks for your feedback!
I think we should highlight the poetry version requirement in the README to avoid confusion in this regard and also add python 3.10 to the CI pipeline as you suggested!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants