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

Bump required Python 3.7 -> 3.8, drop TF1 support #1668

Merged
merged 9 commits into from
Dec 17, 2024
Merged

Conversation

NeoLegends
Copy link
Collaborator

Closes #1326

Opening this to run CI and such.

@NeoLegends NeoLegends self-assigned this Dec 13, 2024
@NeoLegends NeoLegends requested review from albertz and a team as code owners December 13, 2024 10:51
@NeoLegends
Copy link
Collaborator Author

NeoLegends commented Dec 13, 2024

By dropping support for 3.7 we also drop support for TF1.

It is my understanding that some folks might still be using TF1.x for some of their experiments. However, these experiments are probably also using ancient RETURNN versions, so I believe it's ok to drop support.

@albertz WDYT?

CHANGELOG.md Outdated Show resolved Hide resolved
@albertz

This comment was marked as resolved.

Markdown is easier to edit for many folks as it has broader usage outside the python docs ecosystem.
@albertz
Copy link
Member

albertz commented Dec 13, 2024

By dropping support for 3.7 we also drop support for TF1.

Why?

CHANGELOG.md Show resolved Hide resolved
@NeoLegends
Copy link
Collaborator Author

Why?

TF1 never supported Python > 3.7. See the CI log. There are no pip packages available for TF1 for 3.8 and onwards.

@albertz
Copy link
Member

albertz commented Dec 13, 2024

Please check with @curufinwe if dropping TF1 is ok.

@albertz albertz requested a review from curufinwe December 13, 2024 12:30
@albertz albertz changed the title Bump required Python 3.7 -> 3.8 Bump required Python 3.7 -> 3.8, drop TF1 support Dec 13, 2024
README.rst Outdated
Dependencies
============

pip dependencies are listed in ``requirements.txt`` and ``requirements-dev``, although some parts of the code may require additional dependencies (e.g. ``librosa``, ``resampy``) on-demand.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pip dependencies are listed in ``requirements.txt`` and ``requirements-dev``, although some parts of the code may require additional dependencies (e.g. ``librosa``, ``resampy``) on-demand.
Pip dependencies are listed in ``requirements.txt`` and ``requirements-dev``,
although some parts of the code may require additional dependencies (e.g. ``librosa``, ``resampy``) on-demand.
For TensorFlow-based setups, RETURNN requires TF >=2.
For PyTorch-based setups, RETURNN requires Torch >=1.

(I actually don't really know our min required Torch version... But this here would be some good starting point.)

@albertz
Copy link
Member

albertz commented Dec 13, 2024

In the failing test, can you filter out [IWE] tensorflow/ from stdout? I think that should fix the test.

@albertz
Copy link
Member

albertz commented Dec 13, 2024

Ah, in the failing test checking the stdout, the SyntaxWarnings are also new. Let's either fix the syntax warning or filter those out as well. Fixing the SyntaxWarning, there is actually a separate PR #954 for that, maybe let's continue that (it's not totally trivial).

@albertz albertz merged commit d51e906 into master Dec 17, 2024
60 checks passed
@albertz albertz deleted the moritz-py38 branch December 17, 2024 11:25
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.

Drop older Python support
2 participants