MAINT: mappingview check for Python 3.4 #8166

Merged
merged 1 commit into from Mar 1, 2017

Conversation

Projects
None yet
6 participants
Contributor

matthew-brett commented Feb 27, 2017

Python 3.4 does not autoamatically import module abc into
collections, causing an error when checking for
collections.abc.MappingView. Do more explicit import of MappingView
to work round this difference.

See: https://travis-ci.org/MacPython/matplotlib-wheels/jobs/205713297#L582 for
error on Python 3.4.

@phobson

phobson approved these changes Mar 1, 2017

lib/matplotlib/cbook/__init__.py
+try:
+ from collections.abc import MappingView
+except ImportError:
+ MappingView = None
@phobson

phobson Mar 1, 2017

Member

Looks like the except block only applies to python 2.7, is that right?

@anntzer

anntzer Mar 1, 2017

Contributor

2.7 has mappingview (https://docs.python.org/2.7/library/collections.html#collections.MappingView) so I don't know when it can ever be triggered.

(My guess is that below, the test on whether we're running Py3 should just be removed -- we may as well sanitize the case where someone passes dic.itervalues() on Py2 too (well I always thought the whole idea was misguided but let's at least be consistent across Python versions.))

@matthew-brett

matthew-brett Mar 1, 2017

Contributor

So - collections.MappingView is present in all Python versions we support, but collections.abc.MappingView is present only for the Python 3 versions:

Changed in version 3.3: Moved Collections Abstract Base Classes to the collections.abc module. For backwards compatibility, they continue to be visible in this module as well.

https://docs.python.org/3.5/library/collections.html#module-collections

We could use collections.MappingView directly, they haven't been deprecated, I guess.

@story645

story645 Mar 1, 2017

Member

I vote for using collections.MappingView directly then as I think the if MappingView = None is clunky

@matthew-brett

matthew-brett Mar 1, 2017

Contributor

Done.

phobson changed the title from MAINT: mappingview check for Python 3.4 to [MRG+1] MAINT: mappingview check for Python 3.4 Mar 1, 2017

Contributor

matthew-brett commented Mar 1, 2017

Member

phobson commented Mar 1, 2017

@matthew-brett totally. I just wanted to make sure I understood it properly since I don't have a py2.7 installation handy.

lib/matplotlib/cbook/__init__.py
+try:
+ from collections.abc import MappingView
+except ImportError:
+ MappingView = None
@anntzer

anntzer Mar 1, 2017

Contributor

2.7 has mappingview (https://docs.python.org/2.7/library/collections.html#collections.MappingView) so I don't know when it can ever be triggered.

(My guess is that below, the test on whether we're running Py3 should just be removed -- we may as well sanitize the case where someone passes dic.itervalues() on Py2 too (well I always thought the whole idea was misguided but let's at least be consistent across Python versions.))

Contributor

anntzer commented Mar 1, 2017

Side question: Codecov says this line is covered, so how was that never caught before in the CI? (Unfortunately Travis doesn't want to load for me right now.)

Contributor

matthew-brett commented Mar 1, 2017

I'm just guessing, but I suspect that some versions of Python 3.4 do an automatic import of abc in collections, and some do not. Here's an example that does not, on macOS:

$ python3.4
Python 3.4.2 (v3.4.2:ab2c023a9432, Oct  5 2014, 20:42:22) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import collections
>>> collections.abc
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'abc'

That's just a guess, because the behavior above is so for all the Python3.4s I could find.

@matthew-brett matthew-brett MAINT: mappingview check for Python 3.4
Python 3.4 does not automatically import module `abc` into
`collections`, causing an error when checking for
`collections.abc.MappingView`.  Use `collections.MappingView` to work
round this difference.

At some point Python may deprecate `collections.MappingView` in favor of
`collections.abc.MappingView` but we can fix that when it arises.
dfce0b2
@anntzer

anntzer approved these changes Mar 1, 2017

Contributor

anntzer commented Mar 1, 2017

LGTM (conditional on tests passing).

tacaswell added this to the 2.0.1 (next bug fix release) milestone Mar 1, 2017

dstansby changed the title from [MRG+1] MAINT: mappingview check for Python 3.4 to MAINT: mappingview check for Python 3.4 Mar 1, 2017

@dstansby dstansby merged commit c2f675d into matplotlib:master Mar 1, 2017

4 of 5 checks passed

codecov/project/tests 97.74% (target 97.9%)
Details
codecov/patch 100% of diff hit (target 80%)
Details
codecov/project/library 57.1% (+0.13%) compared to 72cea62
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Contributor

dstansby commented Mar 1, 2017

It looks like sanitize_sequence isn't present on 2.0.x, so no need for a backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment