Skip to content

Python 3.9 #3926

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

Merged
merged 9 commits into from
Nov 24, 2020
Merged

Python 3.9 #3926

merged 9 commits into from
Nov 24, 2020

Conversation

cclauss
Copy link
Member

@cclauss cclauss commented Nov 22, 2020

Describe your change:

Migrate to Python 3.9 changing the file extensions of all files that require TensorFlow.

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?
  • Upgrade infrastructure

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@cclauss cclauss requested a review from dhruvmanila as a code owner November 22, 2020 14:26
@ghost ghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghost ghost added tests are failing Do not merge until tests pass and removed awaiting reviews This PR is ready to be reviewed labels Nov 22, 2020
@dhruvmanila
Copy link
Member

TensorFlow will still take some time as they have a few dependencies as well. Also, mypy is not fully compatible with 3.9. So, I think we should wait for a bit. As for the typing errors, we should use the 3.9 with future import, and then once mypy is fully compatible and all our dependencies have upgraded to 3.9, then we will remove the future import and start running mypy and upgrade the CI to 3.9.

@ghost ghost added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Nov 23, 2020
@cclauss
Copy link
Member Author

cclauss commented Nov 23, 2020

My strong preference would be that this repo migrates to the current version of Python and leave the following algorithms out of our test cycle because their dependencies do not yet support Python 3.9. I would much rather ensure that our code is tested on and compatible with the current Python than to continue to wait. Six+ weeks is long enough especially given CPython's long alpha, beta, release candidate cycle.

Note: The following files do not have .py extensions because TensorFlow does not yet support the current Python.

  • dynamic_programming/k_means_clustering_tensorflow.py_tf
  • machine_learning/lstm/lstm_prediction.py_tf
  • neural_network/gan.py_tf
  • neural_network/input_data.py_tf

@dhruvmanila
Copy link
Member

I am a bit hesitant to do monkey patching on an entire repository as that might lead to an unforeseen situation in the future. Also, not everyone is going to have the latest Python installed on their machine let alone use it.

On a side note, any idea as to why the test is taking 18 minutes to download the dependecies?

@cclauss
Copy link
Member Author

cclauss commented Nov 23, 2020

monkey patching on an entire repository

I do not think that changing the file extension on four files constitutes monkey patching on an entire repository.

The build times are about the lack of wheels on some dependencies. Does your build cache help us to resolve that issue or will we always have long builds?

@dhruvmanila
Copy link
Member

Try pushing an empty commit and we will check but even without cache it shouldn't take this much time.

@dhruvmanila
Copy link
Member

Closing and reopening the pull request to trigger workflow runs.

@dhruvmanila dhruvmanila reopened this Nov 24, 2020
@cclauss
Copy link
Member Author

cclauss commented Nov 24, 2020

Pip process is 3 minutes with caching... I can live with that.

@dhruvmanila
Copy link
Member

dhruvmanila commented Nov 24, 2020

There's some problem with the wheels of qiskit-aer: https://github.com/TheAlgorithms/Python/pull/3926/checks?check_run_id=1447346191#step:5:150 otherwise the build would've completed in less than 2 minutes.

@dhruvmanila
Copy link
Member

Let's restrict qiskit to 3.8 as well.
Qiskit is yet to support 3.9: https://github.com/Qiskit/qiskit/blob/master/setup.py#L54
Issue: Qiskit/qiskit#1111

@cclauss
Copy link
Member Author

cclauss commented Nov 24, 2020

Qiskit/qiskit-aer#1016

@dhruvmanila
Copy link
Member

Well then we don't need to restrict qiskit. We will be fine with 3 minutes then.

@cclauss
Copy link
Member Author

cclauss commented Nov 24, 2020

Running on Python 3.9 is different than having a wheel for Python 3.9. The former means that it does not work. The latter means that it works but the install time is long. The tests for all of our quantum algorithms run and pass on Python 3.9. The long install times are dealt with by your pip cache.

@ghost ghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss cclauss merged commit 3fdbf97 into TheAlgorithms:master Nov 24, 2020
@cclauss cclauss deleted the Python_3.9 branch November 24, 2020 11:41
stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
* Upgrade to Python 3.9

* pip install wheel for faster builds

* updating DIRECTORY.md

* requirements.txt: tensorflow; python_version < '3.9'

* keras requires tensorflow

* Rename lstm_prediction.py to lstm_prediction.py_tf

* Update requirements.txt

* updating DIRECTORY.md

* Update requirements.txt

Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Co-authored-by: Dhruv Manilawala <[email protected]>
peRFectBeliever pushed a commit to peRFectBeliever/Python that referenced this pull request Apr 1, 2021
* Upgrade to Python 3.9

* pip install wheel for faster builds

* updating DIRECTORY.md

* requirements.txt: tensorflow; python_version < '3.9'

* keras requires tensorflow

* Rename lstm_prediction.py to lstm_prediction.py_tf

* Update requirements.txt

* updating DIRECTORY.md

* Update requirements.txt

Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Co-authored-by: Dhruv Manilawala <[email protected]>
Panquesito7 pushed a commit to Panquesito7/Python that referenced this pull request May 13, 2021
* Upgrade to Python 3.9

* pip install wheel for faster builds

* updating DIRECTORY.md

* requirements.txt: tensorflow; python_version < '3.9'

* keras requires tensorflow

* Rename lstm_prediction.py to lstm_prediction.py_tf

* Update requirements.txt

* updating DIRECTORY.md

* Update requirements.txt

Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Co-authored-by: Dhruv Manilawala <[email protected]>
shermanhui pushed a commit to shermanhui/Python that referenced this pull request Oct 22, 2021
* Upgrade to Python 3.9

* pip install wheel for faster builds

* updating DIRECTORY.md

* requirements.txt: tensorflow; python_version < '3.9'

* keras requires tensorflow

* Rename lstm_prediction.py to lstm_prediction.py_tf

* Update requirements.txt

* updating DIRECTORY.md

* Update requirements.txt

Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Co-authored-by: Dhruv Manilawala <[email protected]>
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.

2 participants