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

Drop python2 support #1819

Merged
merged 11 commits into from Feb 25, 2020
4 changes: 2 additions & 2 deletions .travis.yml
Expand Up @@ -5,11 +5,11 @@ python:
services:
- docker
env:
# - UBUNTU_VER=14.04 - problems with atari-py
- PY_VER=2.7
- PY_VER=3.5.6
- PY_VER=3.6.8
- PY_VER=3.7.3
- PY_VER=3.8.1


install: "" # so travis doesn't do pip install requirements.txt
script:
Expand Down
4 changes: 4 additions & 0 deletions README.rst
Expand Up @@ -160,6 +160,10 @@ We are using `pytest <http://doc.pytest.org>`_ for tests. You can run them via:

What's new
==========
- 2020-02-21 (v 0.17.0)
- Drop python 2 support
+ Add python 3.8 build

- 2020-02-09 (v 0.16.0)
+ EnvSpec API change - remove tags field (retro-active version bump, the changes are actually already in the codebase since 0.15.5 - thanks @wookayin for keeping us in check!)

Expand Down
2 changes: 1 addition & 1 deletion gym/version.py
@@ -1 +1 @@
VERSION = '0.16.0'
VERSION = '0.17.0'
2 changes: 1 addition & 1 deletion py.Dockerfile
Expand Up @@ -19,7 +19,7 @@ RUN pip install pytest pytest-forked lz4

COPY . /usr/local/gym/
WORKDIR /usr/local/gym/
RUN [ "$PYTHON_VER" != "2.7" ] && pip install .[all] || pip install .
RUN [ "$PYTHON_VER" != "3.8.1" ] && pip install .[all] || pip install .

ENTRYPOINT ["/usr/local/gym/bin/docker_entrypoint"]
CMD ["pytest","--forked"]
3 changes: 1 addition & 2 deletions setup.py
Expand Up @@ -45,11 +45,10 @@
tests_require=['pytest', 'mock'],
python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*',
Copy link
Contributor

@tvalentyn tvalentyn Feb 26, 2020

Choose a reason for hiding this comment

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

I think you'd want to update python_requires as well, otherwise as soon as you add Python code that is not Py2-compatible, you will break Python 2 users who don't set an upper bound on your dependency (gym>=0.10), or expect compatibility w.r.t. semantic versioning (gym>=0.15, <1.0 for example) - note that this is a minor version change as per VERSION = '0.17.0'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @cclauss , I've seen your drive-by comments on quite a few projects re: Python 3 migration. FYI - this is a common error when dropping Py2. Another project I saw this: hamcrest/PyHamcrest#131. Perhaps this is something to keep an eye on in the projects that you come across.

Copy link
Contributor

@cclauss cclauss Feb 26, 2020

Choose a reason for hiding this comment

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

I would use the python_requires=">=3.5" approach when dropping legacy Python.

Like #1827 has done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, fully agree; I meant that more projects should be following this pattern, and I thought perhaps you are in the position to raise awareness of the proper way to drop Py2 support in other projects.

classifiers=[
'Programming Language :: Python :: 2',
'Programming Language :: Python :: 2.7',
'Programming Language :: Python :: 3',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
],
)