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

Updated dependencies for Python 3.10 #2376

Merged
merged 39 commits into from
Jan 11, 2022

Conversation

hydrobeam
Copy link
Member

@hydrobeam hydrobeam commented Dec 16, 2021

Overview: What does this pull request change?

Changes a line in pyproject.toml to
python = ">=3.7,<3.11"
from:
python = ^3.7

And then ran poetry update.

Motivation and Explanation: Why and how do your changes improve the library?

I tried running manim on python 3.10 with poetry and noticed that it was only failing on the scipy install, particularly in that it was installing scipy 1.63, not the latest scipy, which has support and binary wheels for python 3.10. I first tried to change this by doing python = ">=3.7"

But then got this error:

  The current project's Python requirement (>=3.7) is not compatible with some of the required packages Python requirement:
    - flake8-pytest-style requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0
    - flake8-pytest-style requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0

  Because no versions of flake8-pytest-style match >1.5.0,<1.5.1 || >1.5.1,<2.0.0
   and flake8-pytest-style (1.5.0) requires Python >=3.6,<4.0, flake8-pytest-style is forbidden.
  So, because flake8-pytest-style (1.5.1) requires Python >=3.6,<4.0
   and manim depends on flake8-pytest-style (^1.5.0), version solving failed.

So i just specified that it must be less than python 3.11 and then ran poetry update and scipy was updated in the process. Then, I could successfully run a manim scene.

Further Information and Comments

I'm not sure what the ^ symbol means, and spent a long time trying to figure out to no avail. I don't think it's particularly important, but I could be wrong.

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

@hydrobeam hydrobeam added the maintenance refactoring, typos, removing clutter/dead code, and other code quality improvements label Dec 16, 2021
@naveen521kk
Copy link
Member

I'm not sure what the ^ symbol means, and spent a long time trying to figure out to no avail. I don't think it's particularly important, but I could be wrong.

See https://python-poetry.org/docs/dependency-specification/#caret-requirements. ^3.7 means the same as >=3.7,<4.0. I'm not sure why poetry error's out though.

@marcin-serwin
Copy link
Collaborator

marcin-serwin commented Dec 24, 2021

I've also encountered problems with this, and I think I understand why the change is important, so I'll describe it here for the sake of completeness:

  • scipy 1.6.1 specifies python >=3.7 as its requirement, but it does not work with python 3.10
  • scipy 1.7.3 specifies python >=3.7,<3.11 as its requirement, and it does work with python 3.10

This means that with python = ^3.7 requirement in the project poetry cannot install newer scipy versions, because 1.6.1 is the last version that claims to work with arbitrarily high python versions. Thus:

  • requirement python = ^3.7 is able to satisfy all requirements but fails to install due to scipy 1.6.1 incorrectly stating that it works with python 3.10.
  • requirement python >=3.7 makes it impossible to satisfy all requirements because flake8-pytest-style requires python <=4.0. This is the error described in motivation section of this PR.
  • finally, requirement python >=3.7,<3.11 is able to satisfy all requirements and allows installing the newest version of scipy that works correctly with python 3.10.

Hope this helps.

@christopher-besch
Copy link
Member

Apparently steps.cache-vars.outputs.poetry-venv-dir isn't defined. I guess poetry config virtualenvs.path isn't returning anything.
The weird thing is that it seems to be working on my machine...
image

Ah, found it:
image
Just scroll up a bit. Apparently cleo isn't part of Python3.10 anymore.
Why did that one not crash the CI? We should look into this.

@christopher-besch
Copy link
Member

Ok, looks like Poetry is using a new installer. I've removed all other dependencies and added that installer.

@christopher-besch
Copy link
Member

I think I've fixed your problem. But right now the installation of Manim fails for Python3.10.

@marcin-serwin
Copy link
Collaborator

marcin-serwin commented Dec 26, 2021

The build fails, because poetry is constrained to old versions of grpcio:

manim/pyproject.toml

Lines 46 to 47 in 7c7bd2b

grpcio = { version = "1.33.*", optional = true }
grpcio-tools = { version = "1.33.*", optional = true }

Changing these lines to ^1.43.0 and running poetry update should fix the problem.

On a related note: constraints with * will break sooner or later. Since the pyproject.toml is being updated anyway, we can also change these. So e.g.

scipy = "*"

gets changed to

scipy = "^1.7.3"

where 1.7.3 is the newest version of scipy available on https://pypi.org/. poetry can do this automatically by running poetry add scipy@latest.

@marcin-serwin
Copy link
Collaborator

poetry update needs to be run when changing dependencies.

@christopher-besch
Copy link
Member

I'm getting old...

pyproject.toml Outdated Show resolved Hide resolved
christopher-besch and others added 2 commits December 26, 2021 15:22
Co-authored-by: Marcin Serwin <marcin.serwin0@protonmail.com>
@christopher-besch
Copy link
Member

Finally, thanks a lot @marcin-serwin!

@Darylgolden
Copy link
Member

After resolving the merge conflict, will the PR be ready?

@christopher-besch
Copy link
Member

What the hell happened to the history, did I screw this up?

@naveen521kk
Copy link
Member

What the hell happened to the history, did I screw this up?

I fixed it by clicking a single button 😄

@christopher-besch
Copy link
Member

christopher-besch commented Jan 10, 2022

What the hell happened to the history, did I screw this up?

I fixed it by clicking a single button smile

Which button?

And thanks for killing the CI, I totally wasn't anxiously waiting for the result of whether I managed to finally clean up this dependency mess. Ahhhh ;)

@naveen521kk
Copy link
Member

Which button?

Update branch button which GitHub shows.

And thanks for killing the CI, I totally wasn't anxiously waiting for the result of whether I managed to finally clean up this dependency mess. Ahhhh ;)

Sorry!

Copy link
Member

@Darylgolden Darylgolden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@naveen521kk naveen521kk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@naveen521kk naveen521kk merged commit 00f0134 into ManimCommunity:main Jan 11, 2022
@hydrobeam hydrobeam deleted the new_scipy branch January 11, 2022 12:29
@behackl behackl changed the title Update Dependencies for Python3.10 Updated dependencies for Python 3.10 Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance refactoring, typos, removing clutter/dead code, and other code quality improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants