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.

Sign up for GitHub

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

Jump to bottom

Remove py2 compat #2645

Merged
merged 13 commits into from
Jan 25, 2019
Merged

Remove py2 compat #2645

merged 13 commits into from
Jan 25, 2019

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented Jan 3, 2019

I was feeling particularly zealous today so I decided to see what it would take to strip out all the Python 2 compatibility code in xarray. I expect some will feel its too soon to merge this so I'm mostly putting this up for show-and-tell and to highlight some of the knots we've tied ourselves into over the years.

shoyer
shoyer approved these changes Jan 3, 2019
View reviewed changes
Copy link
Member

@shoyer shoyer left a comment

Choose a reason for hiding this comment

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

we could also ditch (or save for a later clean-up):

  • our __future__ imports
  • unicode literals (u'something' -> 'something')

xarray/core/formatting.py Outdated Show resolved Hide resolved
@jhamman
Copy link
Member Author

jhamman commented Jan 3, 2019

Was I not ambition enough 😉? It's only January 3 after all. I was really trying not to over do it.

Anyhow, I'm happy to include the future and unicode removals here.

@fmaussion
Copy link
Member

I expect some will feel its too soon to merge this

I never wrote py2 code myself so I don't think I'm of the "conservative" type, but I wonder if this PR will make it harder to provide bugfix cherry-picks on the hypothetical py2 compatible 0.11.x branch? Or do we want to ditch this possibility right away and simply focus on the bright py3-only future?

@jhamman
Copy link
Member Author

jhamman commented Jan 4, 2019

@fmaussion - you've articulated why I expected some friction here. I'm happy to hold off on this until we're sure 0.11 is clear of nasty bugs. @shoyer just went through the back-porting process so maybe he can comment on his appetite for dealing with this sort of thing.

@jhamman
Copy link
Member Author

jhamman commented Jan 6, 2019

@shoyer - at this point, do you anticipate another bug-fix release for 0.11? If not, I'm now leaning toward merging this to avoid too many merge conflicts.

@shoyer
Copy link
Member

shoyer commented Jan 6, 2019

I think we should wait for a few weeks. There are two regressions that were recently fixed and that we may want to back-port: #2648 and #2651.

@max-sixty
Copy link
Collaborator

What are our thoughts on merging this? I am sensitive that merge conflicts are growing

@jhamman
Copy link
Member Author

jhamman commented Jan 24, 2019

I think we should issue a back-port bug fix (0.11.3) and then merge this asap. The two issues @shoyer referenced above have been merged so, as far as I understand, there is nothing blocking the bugfix release at this point.

@shoyer
Copy link
Member

shoyer commented Jan 24, 2019 via email

@shoyer
Copy link
Member

shoyer commented Jan 24, 2019

See #2704 for a PR with release notes for 0.11.3. But I agree, there should be nothing stopping this from being merged now.

@jhamman
Copy link
Member Author

jhamman commented Jan 24, 2019

I'll sort out the conflicts today and get this merged ASAP.

@dcherian
Copy link
Contributor

Also see #2574

@jhamman jhamman mentioned this pull request Jan 25, 2019
Closed
@jhamman
Copy link
Member Author

jhamman commented Jan 25, 2019

I plan to merge this tomorrow unless I hear otherwise.

shoyer
shoyer reviewed Jan 25, 2019
View reviewed changes
xarray/core/coordinates.py Outdated Show resolved Hide resolved
xarray/core/indexes.py Outdated Show resolved Hide resolved
@shoyer
Copy link
Member

shoyer commented Jan 25, 2019

I found a couple of minor issues, otherwise it looks good!

@shoyer shoyer merged commit aabda43 into pydata:master Jan 25, 2019
@shoyer
Copy link
Member

shoyer commented Jan 25, 2019

thanks @jhamman !

@jhamman jhamman deleted the remove_py2_compat branch January 25, 2019 16:46
@spencerkclark spencerkclark mentioned this pull request Jan 27, 2019
Merged
3 tasks
dcherian pushed a commit that referenced this pull request Jan 30, 2019
Squashed commit of the following:
56bc724 
commit 620b946
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Sun Jan 27 13:02:03 2019 -0800

    Fix test failures / warnings for pandas 0.24 (#2720)

    * Fix test failures / warnings for pandas 0.24

    Fixes GH2717

    * Brief doc note

    * Comment on name order

commit 882deac
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Sun Jan 27 09:11:47 2019 -0800

    DOC: refresh whats-new for 0.11.3 / 0.12.0 (#2718)

commit 8ca8efe
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Sat Jan 26 10:14:49 2019 -0800

    Update environment for doc build (#2708)

    * Update environment for doc build

    We were pinning very old versions for most of these packages. This should
    fix the failures on ReadTheDocs.

    * Build fixes

commit 2e99c7d
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Fri Jan 25 13:52:17 2019 -0800

    Print full environment fron conf.py (#2709)

    This should make it easier to debug the doc build environment.

commit cc5015a
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Fri Jan 25 11:55:07 2019 -0800

    BUG: ensure indexes are reset when coords are modified (#2707)

    This was introduced by the recent indexes refactor, but never made it into a
    release.

commit aabda43
Author: Joe Hamman <jhamman@ucar.edu>
Date:   Fri Jan 25 08:38:45 2019 -0800

    Remove py2 compat (#2645)

    * strip out PY2 compat code from pycompat.py

    * isort

    * remove 2 unused imports

    * remove extra import

    * no more future

    * no unicode literals

    * no more ReprMixin

    * cleanup merge

    * remove deprecated imports from collections

    * 2 more cleanups from shoyer
dcherian pushed a commit that referenced this pull request Jan 30, 2019
Squashed commit of the following:
67aa721 
commit 37a947a
Author: Deepak Cherian <dcherian@users.noreply.github.com>
Date:   Wed Jan 30 09:20:57 2019 -0800

    Refactor plot utils (#2670)

    * Refactor out utility functions.

    * facetgrid refactor

    1. refactor out _easy_facetgrid
    2. Combine map_dataarray_line with map_dataarray

    * flake8

    * Refactor out colorbar making to plot.utils._add_colorbar

    * Refactor out cmap_params, cbar_kwargs processing

    * Back to map_dataarray_line

    * lint

    * small rename

    * review comment.

    * Move _infer_line_data back.

commit fd2552a
Author: Spencer Clark <spencerkclark@gmail.com>
Date:   Wed Jan 30 11:45:09 2019 -0500

    Enable subtracting a scalar cftime.datetime object from a CFTimeIndex (#2672)

    * Enable subtracting a scalar cftime.datetime object from a CFTimeIndex

    * lint

    * Test cftime.datetime minus CFTimeIndex as well

    * Fix cftime minus CFTimeIndex

commit e8bf4bf
Author: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com>
Date:   Wed Jan 30 02:02:22 2019 +0000

    Bugfix for line plot axes (#2726)

    * Fixed logic for setting line data

    * Added tests to check line data matches values of correct coords

    * Recorded bugfix for line plots

    * Update doc/whats-new.rst

    Co-Authored-By: TomNicholas <35968931+TomNicholas@users.noreply.github.com>

    * Update doc/whats-new.rst

    Co-Authored-By: TomNicholas <35968931+TomNicholas@users.noreply.github.com>

commit 620b946
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Sun Jan 27 13:02:03 2019 -0800

    Fix test failures / warnings for pandas 0.24 (#2720)

    * Fix test failures / warnings for pandas 0.24

    Fixes GH2717

    * Brief doc note

    * Comment on name order

commit 882deac
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Sun Jan 27 09:11:47 2019 -0800

    DOC: refresh whats-new for 0.11.3 / 0.12.0 (#2718)

commit 8ca8efe
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Sat Jan 26 10:14:49 2019 -0800

    Update environment for doc build (#2708)

    * Update environment for doc build

    We were pinning very old versions for most of these packages. This should
    fix the failures on ReadTheDocs.

    * Build fixes

commit 2e99c7d
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Fri Jan 25 13:52:17 2019 -0800

    Print full environment fron conf.py (#2709)

    This should make it easier to debug the doc build environment.

commit cc5015a
Author: Stephan Hoyer <shoyer@gmail.com>
Date:   Fri Jan 25 11:55:07 2019 -0800

    BUG: ensure indexes are reset when coords are modified (#2707)

    This was introduced by the recent indexes refactor, but never made it into a
    release.

commit aabda43
Author: Joe Hamman <jhamman@ucar.edu>
Date:   Fri Jan 25 08:38:45 2019 -0800

    Remove py2 compat (#2645)

    * strip out PY2 compat code from pycompat.py

    * isort

    * remove 2 unused imports

    * remove extra import

    * no more future

    * no unicode literals

    * no more ReprMixin

    * cleanup merge

    * remove deprecated imports from collections

    * 2 more cleanups from shoyer
dcherian pushed a commit to yohai/xarray that referenced this pull request Jan 30, 2019
Merge remote-tracking branch 'upstream/master' into yohai-ds_scatter
bce0152 
* upstream/master:
  Refactor plot utils (pydata#2670)
  Enable subtracting a scalar cftime.datetime object from a CFTimeIndex (pydata#2672)
  Bugfix for line plot axes (pydata#2726)
  Fix test failures / warnings for pandas 0.24 (pydata#2720)
  DOC: refresh whats-new for 0.11.3 / 0.12.0 (pydata#2718)
  Update environment for doc build (pydata#2708)
  Print full environment fron conf.py (pydata#2709)
  BUG: ensure indexes are reset when coords are modified (pydata#2707)
  Remove py2 compat (pydata#2645)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shoyer shoyer shoyer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jhamman @fmaussion @shoyer @max-sixty @dcherian