Skip to content

Python 3.9 - #3926

Merged
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9
Nov 24, 2020
Merged

Python 3.9#3926
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9

Conversation

@cclauss

@cclausscclauss commented Nov 22, 2020

Copy link
Copy Markdown
Member

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}.

@ghostghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghostghost 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
Copy Markdown
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.

@ghostghost 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

cclauss commented Nov 23, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@dhruvmanila

Copy link
Copy Markdown
Member

Closing and reopening the pull request to trigger workflow runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

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

@dhruvmanila

dhruvmanila commented Nov 24, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
MemberAuthor

@dhruvmanila

Copy link
Copy Markdown
Member

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

@cclauss

Copy link
Copy Markdown
MemberAuthor

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.

@ghostghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss
cclauss merged commit 3fdbf97 into TheAlgorithms:masterNov 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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
Sign up for freeto 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

@cclauss@dhruvmanila
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Python 3.9 by cclauss · Pull Request #3926 · TheAlgorithms/Python · GitHub
Skip to content

Python 3.9 - #3926

Merged
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9
Nov 24, 2020
Merged

Python 3.9#3926
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9

Conversation

@cclauss

@cclausscclauss commented Nov 22, 2020

Copy link
Copy Markdown
Member

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}.

@ghostghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghostghost 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
Copy Markdown
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.

@ghostghost 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

cclauss commented Nov 23, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@dhruvmanila

Copy link
Copy Markdown
Member

Closing and reopening the pull request to trigger workflow runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

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

@dhruvmanila

dhruvmanila commented Nov 24, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
MemberAuthor

@dhruvmanila

Copy link
Copy Markdown
Member

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

@cclauss

Copy link
Copy Markdown
MemberAuthor

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.

@ghostghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss
cclauss merged commit 3fdbf97 into TheAlgorithms:masterNov 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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
Sign up for freeto 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

@cclauss@dhruvmanila
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Python 3.9 by cclauss · Pull Request #3926 · TheAlgorithms/Python · GitHub
Skip to content

Python 3.9 - #3926

Merged
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9
Nov 24, 2020
Merged

Python 3.9#3926
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9

Conversation

@cclauss

@cclausscclauss commented Nov 22, 2020

Copy link
Copy Markdown
Member

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}.

@ghostghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghostghost 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
Copy Markdown
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.

@ghostghost 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

cclauss commented Nov 23, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@dhruvmanila

Copy link
Copy Markdown
Member

Closing and reopening the pull request to trigger workflow runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

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

@dhruvmanila

dhruvmanila commented Nov 24, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
MemberAuthor

@dhruvmanila

Copy link
Copy Markdown
Member

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

@cclauss

Copy link
Copy Markdown
MemberAuthor

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.

@ghostghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss
cclauss merged commit 3fdbf97 into TheAlgorithms:masterNov 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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
Sign up for freeto 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

@cclauss@dhruvmanila
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Python 3.9 by cclauss · Pull Request #3926 · TheAlgorithms/Python · GitHub
Skip to content

Python 3.9 - #3926

Merged
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9
Nov 24, 2020
Merged

Python 3.9#3926
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9

Conversation

@cclauss

@cclausscclauss commented Nov 22, 2020

Copy link
Copy Markdown
Member

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}.

@ghostghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghostghost 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
Copy Markdown
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.

@ghostghost 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

cclauss commented Nov 23, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@dhruvmanila

Copy link
Copy Markdown
Member

Closing and reopening the pull request to trigger workflow runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

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

@dhruvmanila

dhruvmanila commented Nov 24, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
MemberAuthor

@dhruvmanila

Copy link
Copy Markdown
Member

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

@cclauss

Copy link
Copy Markdown
MemberAuthor

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.

@ghostghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss
cclauss merged commit 3fdbf97 into TheAlgorithms:masterNov 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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
Sign up for freeto 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

@cclauss@dhruvmanila
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Python 3.9 by cclauss · Pull Request #3926 · TheAlgorithms/Python · GitHub
Skip to content

Python 3.9 - #3926

Merged
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9
Nov 24, 2020
Merged

Python 3.9#3926
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9

Conversation

@cclauss

@cclausscclauss commented Nov 22, 2020

Copy link
Copy Markdown
Member

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}.

@ghostghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghostghost 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
Copy Markdown
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.

@ghostghost 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

cclauss commented Nov 23, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@dhruvmanila

Copy link
Copy Markdown
Member

Closing and reopening the pull request to trigger workflow runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

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

@dhruvmanila

dhruvmanila commented Nov 24, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
MemberAuthor

@dhruvmanila

Copy link
Copy Markdown
Member

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

@cclauss

Copy link
Copy Markdown
MemberAuthor

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.

@ghostghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss
cclauss merged commit 3fdbf97 into TheAlgorithms:masterNov 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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
Sign up for freeto 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

@cclauss@dhruvmanila
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Python 3.9 by cclauss · Pull Request #3926 · TheAlgorithms/Python · GitHub
Skip to content

Python 3.9 - #3926

Merged
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9
Nov 24, 2020
Merged

Python 3.9#3926
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9

Conversation

@cclauss

@cclausscclauss commented Nov 22, 2020

Copy link
Copy Markdown
Member

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}.

@ghostghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghostghost 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
Copy Markdown
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.

@ghostghost 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

cclauss commented Nov 23, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@dhruvmanila

Copy link
Copy Markdown
Member

Closing and reopening the pull request to trigger workflow runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

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

@dhruvmanila

dhruvmanila commented Nov 24, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
MemberAuthor

@dhruvmanila

Copy link
Copy Markdown
Member

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

@cclauss

Copy link
Copy Markdown
MemberAuthor

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.

@ghostghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss
cclauss merged commit 3fdbf97 into TheAlgorithms:masterNov 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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
Sign up for freeto 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

@cclauss@dhruvmanila
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Python 3.9 by cclauss · Pull Request #3926 · TheAlgorithms/Python · GitHub
Skip to content

Python 3.9 - #3926

Merged
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9
Nov 24, 2020
Merged

Python 3.9#3926
cclauss merged 9 commits into
TheAlgorithms:masterfrom
cclauss:Python_3.9

Conversation

@cclauss

@cclausscclauss commented Nov 22, 2020

Copy link
Copy Markdown
Member

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}.

@ghostghost added the awaiting reviews This PR is ready to be reviewed label Nov 22, 2020
@ghostghost 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
Copy Markdown
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.

@ghostghost 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

cclauss commented Nov 23, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@dhruvmanila

Copy link
Copy Markdown
Member

Closing and reopening the pull request to trigger workflow runs.

@cclauss

Copy link
Copy Markdown
MemberAuthor

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

@dhruvmanila

dhruvmanila commented Nov 24, 2020

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
MemberAuthor

@dhruvmanila

Copy link
Copy Markdown
Member

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

@cclauss

Copy link
Copy Markdown
MemberAuthor

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.

@ghostghost removed the awaiting reviews This PR is ready to be reviewed label Nov 24, 2020
@cclauss
cclauss merged commit 3fdbf97 into TheAlgorithms:masterNov 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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
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 <dhruvmanila@gmail.com>
Sign up for freeto 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

@cclauss@dhruvmanila