Skip to content

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented Aug 13, 2019

Drops support for Python 2. Closes #393.

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • Docs build locally (e.g., run tox -e docs)
  • AppVeyor and Travis CI passes
  • Test coverage is 100% (Coveralls passes)

@jeromekelleher
Copy link
Member

Thanks for doing this @jhamman! We dropped Python 2 support in tskit in this PR, which might be useful in outlining the steps required. In particular, pyupgrade seems really useful.

@pep8speaks
Copy link

pep8speaks commented Aug 13, 2019

Hello @jhamman! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-10-18 20:13:27 UTC

@jhamman
Copy link
Member Author

jhamman commented Aug 13, 2019

Thanks @jeromekelleher - I've implemented the following steps

  1. changed setup.py to include python_requires
  2. removed Python 2 special cases (e.g. if PY2: ...)
  3. run pyupgrade
  4. run isort -rc --atomic . (helped with step 5)
  5. removed zarr.compat, imports to py3 modules

Tests are passing locally for me but we'll wait to see what coveralls/travis reports. I also need to add a note in docs/release.rst

Joseph Hamman added 2 commits August 13, 2019 10:51
@jhamman
Copy link
Member Author

jhamman commented Sep 25, 2019

@zarr-developers/core-devs - just pinging everyone on this. Luckily, we don't have any merge conflicts yet but it would still be good to get this reviewed and merged before things get messy.

@jhamman
Copy link
Member Author

jhamman commented Oct 18, 2019

@zarr-developers/core-devs - one more ping here. This needs a review.

Copy link
Member

@alimanfoo alimanfoo left a comment

Choose a reason for hiding this comment

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

Looks good to me! Only noticed one line in the tox config file which could go as well.

@jhamman jhamman merged commit e7708c9 into zarr-developers:master Oct 19, 2019
@jhamman jhamman deleted the drop_py2 branch October 19, 2019 04:10
@alimanfoo
Copy link
Member

alimanfoo commented Oct 19, 2019 via email

@jakirkham jakirkham mentioned this pull request Mar 3, 2020
@Carreau Carreau added this to the v2.4 milestone Sep 9, 2020
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.

When will Zarr drop Python 2 support?
5 participants