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

Make qualang-tools compatible with python 3.12 #249

Merged
merged 16 commits into from
Dec 10, 2024
Merged

Make qualang-tools compatible with python 3.12 #249

merged 16 commits into from
Dec 10, 2024

Conversation

TheoLaudatQM
Copy link
Contributor

Make qualang-tools compatible with python 3.12

@TheoLaudatQM TheoLaudatQM requested a review from yomach November 21, 2024 14:15
Copy link

github-actions bot commented Nov 21, 2024

Unit Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit ad0e6ac.

♻️ This comment has been updated with latest results.

@TheoLaudatQM
Copy link
Contributor Author

Hey @yomach, now that qm-qua support python 3.12, we need to release the constraint on qualang-tools as well.
Is there anything more that updating the pyproject.toml file and test everything?

@TheoLaudatQM
Copy link
Contributor Author

Also there is a dependency issue with scipy:

  • scipy 1.10.1 supports python 3.8 but not 3.12
  • scipy 1.11.0 supports python 3.12 but not 3.8

Do you know how to solve this, since qm-qua still supports python 3.8 - 3.12?

@yomach
Copy link
Collaborator

yomach commented Nov 24, 2024

@TheoLaudatQM Yes, see how I solved it.
I also fixed/improved a few others, and did poetry update.
The only thing left is to test that everything works

@TheoLaudatQM
Copy link
Contributor Author

Nice, thanks a lot Yoav!
If think that the unit tests were performed with Python 3.10, is there a way to automate the tests to use both Python 3.8 and 3.12?

@yomach
Copy link
Collaborator

yomach commented Nov 25, 2024

Yes, but for that I would ask for @yonatanrqm help

@nulinspiratie
Copy link
Contributor

We test for multiple python versions in QUAM as well, I'll add it here too

@nulinspiratie
Copy link
Contributor

By the way, should we continue supporting 3.8? Python stopped support for 3.8

@yomach
Copy link
Collaborator

yomach commented Nov 27, 2024

By the way, should we continue supporting 3.8? Python stopped support for 3.8

We will drop support for Python 3.8 soon (either qm-qua 1.2.2 or 1.2.3)

@nulinspiratie
Copy link
Contributor

There are a number of new linting errors (not real errors) due to pylint needing to be upgraded for Python 3.12. I ignore E231 but there are several more. Should we ignore all of them or resolve them? @yomach @TheoLaudatQM

@TheoLaudatQM
Copy link
Contributor Author

I would naively say that we should resolve it since it is a requirement for python 3.12, even if it is just linting and without thinking too much I am guessing that there is a good reason behind adding these additional linting checks, but I also don't know more than that so I let you guys decide since you have more experience than me

@nulinspiratie
Copy link
Contributor

I would naively say that we should resolve it since it is a requirement for python 3.12, even if it is just linting and without thinking too much I am guessing that there is a good reason behind adding these additional linting checks, but I also don't know more than that so I let you guys decide since you have more experience than me

I propose we do it in two stages. First we ignore the linting rules and verify that all tests are still working, then merge.
In a follow-up PR we fix all the linting issues. This way the changes in this PR don't explode, and the changes in the other PR are solely focused on linting issues

@yomach
Copy link
Collaborator

yomach commented Dec 3, 2024

I would just fix it, most of them are simple whitespace anyway.
Ignoring is a sure way to never fix it :)
I can get to it next week if you don't have the time this week.

@nulinspiratie
Copy link
Contributor

Ok that works for me as well. I will probably have time friday to handle this

@nulinspiratie
Copy link
Contributor

@yomach @TheoLaudatQM I can't believe it, it finally worked 🥲
I fixed many linting errors.
Additionally I had to use the specific Flake8 version 5.0.4 rather than the latest version 7.1.1 as this is the latest version that supports Python 3.8. As a result, I had to disable the following flake8 errors: E231, E713, W604, E241, E221, E222, E225 because they are actually erroneous, and are removed in later Flake8 versions. Once we drop support for Python 3.8 we should remove these disabled errors again.

@TheoLaudatQM @yomach ready to merge?

@yomach
Copy link
Collaborator

yomach commented Dec 10, 2024

Nice work!!!
Yes, I'll merge.
I think that soon (next version?) we'll remove Python 3.8, and then we should revisit this lint part.

@yomach yomach merged commit be92404 into main Dec 10, 2024
4 checks passed
@yomach yomach deleted the feat/python3_12 branch December 10, 2024 08:02
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