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

Suppport Python 3.12 #48

Merged
merged 9 commits into from
Apr 30, 2024
Merged

Suppport Python 3.12 #48

merged 9 commits into from
Apr 30, 2024

Conversation

alecandido
Copy link
Member

No description provided.

@alecandido
Copy link
Member Author

Just waiting for qiboteam/qibo#1231 to be merged

@alecandido alecandido requested review from Vinitha-balachandran and yjmaxpayne and removed request for Vinitha-balachandran April 10, 2024 14:00
Copy link

@yjmaxpayne yjmaxpayne left a comment

Choose a reason for hiding this comment

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

Thanks Alessandro for the implementation. I noticed all tests are passing, but the test for python 3.12 is not included. You may wish to change the corresponding workflow in deploy.yml: python-version: [3.9, "3.10", "3.11"] to include 3.12.

@alecandido
Copy link
Member Author

alecandido commented Apr 16, 2024

Thanks Alessandro for the implementation. I noticed all tests are passing, but the test for python 3.12 is not included. You may wish to change the corresponding workflow in deploy.yml: python-version: [3.9, "3.10", "3.11"] to include 3.12.

Done, thanks @yjmaxpayne

EDIT: of course it was done in 3718d5c, after this comment (because I didn't notice the push didn't go through...)

@yjmaxpayne
Copy link

Thanks Alessandro for the implementation. I noticed all tests are passing, but the test for python 3.12 is not included. You may wish to change the corresponding workflow in deploy.yml: python-version: [3.9, "3.10", "3.11"] to include 3.12.

Done, thanks @yjmaxpayne

EDIT: of course it was done in 3718d5c, after this comment (because I didn't notice the push didn't go through...)

Got a runtime error on dependencies for the python 3.12 test.

RuntimeError

  Unable to find installation candidates for kahypar (1.3.5)

  at ~/.local/venv/lib/python3.12/site-packages/poetry/installation/chooser.py:74 in choose_for
       [70](https://github.com/qiboteam/qibotn/actions/runs/8703157064/job/23868729420?pr=48#step:9:71)│ 
       71│             links.append(link)
       [72](https://github.com/qiboteam/qibotn/actions/runs/8703157064/job/23868729420?pr=48#step:9:73)│ 
       73│         if not links:
    →  74│             raise RuntimeError(f"Unable to find installation candidates for {package}")
       [75](https://github.com/qiboteam/qibotn/actions/runs/8703157064/job/23868729420?pr=48#step:9:76)│ 
       76│         # Get the best link
       77│         chosen = max(links, key=lambda link: self._sort_key(package, link))
       [78](https://github.com/qiboteam/qibotn/actions/runs/8703157064/job/23868729420?pr=48#step:9:79)│ 

Cannot install kahypar.

Error: Process completed with exit code 1.

@alecandido
Copy link
Member Author

@alecandido
Copy link
Member Author

alecandido commented Apr 16, 2024

Sorry for the late reply: unfortunately, there is nothing we can do in QiboTN to fix the former error.

kahypar never released support for py3.12 https://pypi.org/project/kahypar/#files (no wheel for py3.12, and no source on PyPI).

So, either we can remove that dependency from Poetry

qibotn/pyproject.toml

Lines 42 to 43 in 3718d5c

# to prevent Quimb warning
kahypar = "^1.3.5"

or we have to wait for them.

However, the dependency is only required for the docs, and it is only making the build failing.
In principle, we could even remove docs from here:

poetry-extras: "--with docs,tests,analysis"

and neglect the problem (but, if we don't fix it, it will make the publishing workflow failing afterward).

@Vinitha-balachandran
Copy link
Contributor

Vinitha-balachandran commented Apr 17, 2024

Sorry for the late reply: unfortunately, there is nothing we can do in QiboTN to fix the former error.

kahypar never released support for py3.12 https://pypi.org/project/kahypar/#files (no wheel for py3.12, and no source on PyPI).

So, either we can remove that dependency from Poetry

qibotn/pyproject.toml

Lines 42 to 43 in 3718d5c

# to prevent Quimb warning
kahypar = "^1.3.5"

or we have to wait for them.
However, the dependency is only required for the docs, and it is only making the build failing. In principle, we could even remove docs from here:

poetry-extras: "--with docs,tests,analysis"

and neglect the problem (but, if we don't fix it, it will make the publishing workflow failing afterward).

Hi Alessandro, I find the lines 42-43 introduced to avoid quimb warning in pyproject.toml file not working (even for the released version). Liwei also finds same problem. This is the warning:

.conda/envs/qibotn_new/lib/python3.12/site-packages/cotengra/hyperoptimizers/hyper.py:33: UserWarning: Couldn't import `kahypar` - skipping from default hyper optimizer and using basic `labels` method instead.

So, I think we may remove the lines 42-43. Other than this warning, the tests will pass with python 3.12

@alecandido
Copy link
Member Author

alecandido commented Apr 29, 2024

Hi Alessandro, I find the lines 42-43 introduced to avoid quimb warning in pyproject.toml file not working (even for the released version). Liwei also finds same problem. This is the warning:

This is definitely expected, since those lines are meant to be evaluated in the CI, but it won't happen in other scenarios, especially for the released version, that fully lost track of the Poetry settings (since only actual dependencies are propagated to the wheels).

If you want to make use of them, you should install with poetry install --with docs (given the group where they are located), or move them up to the actual dependencies (but I would avoid, since we don't strictly need it).

If the goal was to save the warning to the QiboTN user, that was not an appropriate solution, so I agree we should drop them. Instead, if there was a reason to prevent the warning during docs generation, it should be reassessed the original motivation.

EDIT: and since I added myself, and I forgot the motivation, let me remove them

@alecandido alecandido marked this pull request as ready for review April 29, 2024 15:43
@alecandido alecandido requested a review from yjmaxpayne April 29, 2024 15:43
@alecandido alecandido mentioned this pull request Apr 29, 2024
4 tasks
Copy link
Contributor

@BrunoLiegiBastonLiegi BrunoLiegiBastonLiegi left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. Just out of curiosity, how did you handle the issue of kahypar not having wheels for 3.12 in the end?

@alecandido
Copy link
Member Author

Thanks, looks good. Just out of curiosity, how did you handle the issue of kahypar not having wheels for 3.12 in the end?

Removing the dependency on kahypar, that was just saving a warning in the docs generation. We now have the warning back, but Poetry is able to resolve dependencies for py3.12.

Copy link
Contributor

@Vinitha-balachandran Vinitha-balachandran left a comment

Choose a reason for hiding this comment

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

Works fine.

@scarrazza scarrazza merged commit 6814fdb into main Apr 30, 2024
7 checks passed
@scarrazza scarrazza deleted the py3.12 branch April 30, 2024 09:57
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.

5 participants