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

add python==3.13 and drop python==3.8 #1175

Draft
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

Kallinteris-Andreas
Copy link
Collaborator

@Kallinteris-Andreas Kallinteris-Andreas commented Sep 25, 2024

No description provided.

@Kallinteris-Andreas Kallinteris-Andreas marked this pull request as draft September 25, 2024 13:25
@RedTachyon
Copy link
Member

Is there anything forcing us to break compatibility with 3.8? My general intuition is that we can keep it around for now, and once we need any features that were added in 3.9, we drop it without much worry in the next release

@Kallinteris-Andreas
Copy link
Collaborator Author

Generally we drop support for python versions once they become unsupported by the python developers.

@RedTachyon
Copy link
Member

This isn't some dogma that we must always follow. My point is a bit more from first principles - there's a difference between actively and passively supporting some version.
With 1.0, we will actively support python>=3.9,<=3.13, as in we make sure that Gymnasium works with each of those versions.
On the other hand, I want to avoid deliberately "unsupporting" 3.8, i.e. we passively support 3.8 for as long as it makes sense.

FWIW I just checked and I'm pretty sure this PR actually does it the way I'm proposing:

requires-python = ">= 3.8"

If we really want to drop 3.8, we'd bump this to 3.9, so that users cannot (without some weird workarounds) install gymnasium on 3.8. I don't think that makes sense to do if we know that Gymnasium actually runs alright on 3.8.

On the other hand, if we want to keep this "passive" support, removing it from the test suite makes it... awkward. Let's say in 1.1.0 we want to add a feature which uses, say, dict merging {1: 2} | {3: 4}, which was added in 3.9. We shouldn't hesitate to add this - but at that point we should also stop supporting 3.8, even passively, i.e. we actively mark it as incompatible by changing the aforementioned line in pyproject.toml. But if we don't have 3.8 in the test suite, then we won't know when this happens, so we might end up in a weird state where someone (for whatever reason) using 3.8, by default installs a version of Gymnasium that doesn't actually work.

All this to say - we should add 3.13 to the test matrix, and keep a policy that 3.8 compatibility can be broken, but consciously. There is no reason that I see to actually drop support right now, so let's keep it to maintain wider compatibility. We definitely shouldn't do anything halfway, i.e. allow it to be installed but untested.

pyproject.toml Outdated
@@ -13,14 +13,14 @@ authors = [{ name = "Farama Foundation", email = "[email protected]" }]
license = { text = "MIT License" }
keywords = ["Reinforcement Learning", "game", "RL", "AI", "gymnasium"]
classifiers = [
"Development Status :: 4 - Beta", # change to `5 - Production/Stable` when ready
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is done because 1.0 release

@Kallinteris-Andreas
Copy link
Collaborator Author

Most libraries such as numpy also stop supporting python versions as they become unsupported, so some issues may be caused long term, I am not opposed to making an exception for 1.0 specifically to have 3.8 supported, also depends on when 1.0 is actually being released

@pseudo-rnd-thoughts
Copy link
Member

This would break what @RedTachyon is suggesting, but if we completely dropping 3.8 then we should update pyupgrade in pre-commit to --py39-plus which causes a significant number of changes, mainly reducing the number of type imports and to pyright in pyproject.toml with pythonVersion="3.9"

@pseudo-rnd-thoughts
Copy link
Member

Python 3.13 has been delayed by a week due to unexpected performance regressions fyi

@Kallinteris-Andreas
Copy link
Collaborator Author

Waiting for pytorch as usual

@pseudo-rnd-thoughts
Copy link
Member

Mujoco 3.2.5 doesn't support 3.13 yet - google-deepmind/mujoco#2232

@h-vetinari
Copy link

You're also going to have to move to Cython 3.0, because that's the minimum version necessary for CPython 3.13 support, see

mujoco-py = ["mujoco-py >=2.1,<2.2", "cython<3"]

"cython <3",

@saran-t
Copy link

saran-t commented Dec 3, 2024

MuJoCo 3.2.6 has prebuilt wheels for Python 3.13.

@pseudo-rnd-thoughts
Copy link
Member

Thanks @saran-t

We are still waiting on TensorStore that flax uses - google/tensorstore#204
@Kallinteris-Andreas Could we test without the missing modules just to check that everything works. If we can, then we could cut a release with Python 3.13 and specify that this might not work for users that use modules X, Y, Z

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