Cleanups by anntzer · Pull Request #7544 · matplotlib/matplotlib

anntzer · 2016-12-01T17:55:45Z

What I did during a five-hour flight (split into multiple PRs to keep things reviewable)...

Apparently github doesn't show file renames in the diff, in fact examples/user_interfaces/interactive2.py was renamed as examples/user_interfaces/interactive.py and the previous interactive.py (which was marked as non-working since 2010) was removed.

callable came back as a builtin in Py3.2. NotImplemented is a singleton.

NelleV · 2016-12-01T18:43:08Z

This is going to break so many PRs /o\

WeatherGod · 2016-12-01T19:21:47Z

lib/mpl_toolkits/mplot3d/art3d.py


 def get_colors(c, num):
    """Stretch the color argument to provide the required number num"""
+    return np.broadcast_to(


Travis is failing because this method isn't available in older numpys.

WeatherGod · 2016-12-01T19:23:31Z

tools/gh_api.py

            token, = f
            return token
-    except:
+    except OSError:


What about IOError?

Oh, I missed the fact that the two are not synonyms in Py2 :-) Can open raise both OSError and IOError?

Not sure about open(), but the read token, = f could produce an IOError.

also, thinking about it, what if the file was malformed? (e.g., many lines), in which case a simple readline() might actually be more correct as that would result in an IOError rather than what ever the error is when doing something like a, b = 1, 2, 3

I'll just make it Exception.

dopplershift

Wow, this cleans up so many....interesting things. Especially nice to no longer have 3 separate values defined for pi within the code base.

dopplershift · 2016-12-01T20:28:07Z

lib/matplotlib/font_manager.py

                verbose.report("Could not open font file %s" % fpath)
                continue
            try:
-                try:


This probably dates back to before you could actually have an except and finally on the same try.

dopplershift · 2016-12-01T20:30:31Z

setupext.py

-        try:
-            return super(BackendGtkAgg, self).check()
-        except:
-            raise


I think try:...except: raise has to be my favorite of these. WTF?

Actually the whole method override can go away...

dopplershift · 2016-12-01T20:31:01Z

lib/matplotlib/font_manager.py

            try:
                fh = open(fpath, 'rb')
-            except:
+            except OSError:


Might need IOError here as well.

EnvironmentError is what we want, actually.

QuLogic

Nothing too major.

QuLogic · 2016-12-01T20:51:11Z

lib/matplotlib/artist.py

-            if mouseevent.inaxes is None or ax is None or \
-                    mouseevent.inaxes == ax:
+            if (mouseevent.inaxes is None or ax is None
+                    or mouseevent.inaxes == ax):


Operator goes before the line break.

I tend to (very much) favor the newer PEP8 recommendations (https://www.python.org/dev/peps/pep-0008/#should-a-line-break-before-or-after-a-binary-operator) which I think is usually more readable.

I thought it complained on a different PR, but it seems to have passed here? Not too important...

QuLogic · 2016-12-01T20:52:48Z

lib/matplotlib/artist.py

        else:
-            raise ValueError('match must be None, an '
-                             'matplotlib.artist.Artist '
+            raise ValueError('match must be None, an matplotlib.artist.Artist '


QuLogic · 2016-12-01T20:54:23Z

lib/matplotlib/colors.py

    try:
        adata = np.array(data)
-    except:
+    except Exception:


What does it raise other than TypeError and/or ValueError?

type(data).__array__(data) can raise whatever it wants.

Oh, I thought np.array would wrap that sort of stuff; carry on, then.

QuLogic · 2016-12-01T20:57:21Z

lib/matplotlib/image.py

        if x is not None and y is not None:
-            inside = ((x >= xmin) and (x <= xmax) and
-                      (y >= ymin) and (y <= ymax))
+            inside = xmin <= x <= xmax and ymin <= y <= ymax


I know they're all ands, but I'd put parentheses just the same.

I think this line reads quite clearly? There's only one and now.

I know it's redundant, but I like parentheses around the x and y parts; it's not a major thing, though.

I agree with @QuLogic. Extra parentheses can greatly help readability, and in such cases, like this one, they do no harm at all.

QuLogic · 2016-12-01T21:00:40Z

lib/matplotlib/rcsetup.py

+        except KeyError:
+            raise ValueError(
+                'Supported Postscript/PDF font types are %s' %
+                list(six.iterkeys(fonttypes)))


Can you make it list(fonttypes.keys())? Why bother with six for this...

just list(fonttypes)

Even better.

tacaswell · 2016-12-02T03:28:41Z

doc/api/api_changes/code_removal.rst

 removed.

+The ``ArtistInspector.findobj`` method, which was never working due to the lack
+of a ``get_children`` method, has been removed.


what was lacking get_children?

The ArtistInspector class.

QuLogic · 2016-12-03T02:51:18Z

Build's failing; looks like old NumPy don't have np.iterable.

QuLogic · 2016-12-03T02:52:26Z

matplotlib.tests.test_patheffects.test_collection(0, 'collection', *) seems to have genuinely failed on all builds as well.

anntzer · 2016-12-03T03:45:29Z

I'm confused: building numpy 1.7.1 from source shows that it does define np.iterable.

Edit: Ah, stupid implicit relative imports

anntzer · 2016-12-03T03:51:39Z

The other failure is due to the change from / 180 * 3.141592 to deg2rad in _get_text_path_transform, so the new result image is the correct one...

QuLogic · 2016-12-03T04:14:14Z

Need to update the other formats, not just the png.

anntzer · 2016-12-03T05:09:16Z

Done.

tacaswell · 2016-12-03T21:49:18Z

'power cycled' to restart all of the CI

QuLogic · 2016-12-03T22:27:59Z

lib/matplotlib/_backports/numpy.py

+import numpy as np
+
+
+# Copy-pasted from numpy.lib.stride_tricks 1.11.2.


Can you add this comment on all functions? This will help in the future if any others are added here and these need to be removed.

codecov-io · 2016-12-03T22:39:21Z

Current coverage is 62.02% (diff: 58.33%)

Merging #7544 into master will increase coverage by 0.10%

@@             master      #7544   diff @@
==========================================
  Files           173        174     +1   
  Lines         56177      56296   +119   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          34784      34918   +134   
+ Misses        21393      21378    -15   
  Partials          0          0

Powered by Codecov. Last update e66d2f1...afa9344

tacaswell · 2016-12-04T21:53:11Z

restarted the travis test (looks like weird filesystem related failures). The appveyor tests look transient

anntzer added 2 commits December 1, 2016 09:29

Remove nonworking interactive.py.

e197d83

NotImplemented is a singleton.

a8fe0b3

NelleV approved these changes Dec 1, 2016

View reviewed changes

NelleV changed the title ~~Cleanups~~ [MRG+1] Cleanups Dec 1, 2016

WeatherGod requested changes Dec 1, 2016

View reviewed changes

anntzer force-pushed the cleanup branch from 1ceca8f to d036c75 Compare December 1, 2016 20:18

dopplershift approved these changes Dec 1, 2016

View reviewed changes

QuLogic reviewed Dec 1, 2016

View reviewed changes

anntzer force-pushed the cleanup branch from badc714 to f3b39fa Compare December 1, 2016 21:59

WeatherGod approved these changes Dec 1, 2016

View reviewed changes

anntzer force-pushed the cleanup branch from f3b39fa to 44d640b Compare December 2, 2016 01:01

tacaswell reviewed Dec 2, 2016

View reviewed changes

tacaswell added this to the 2.1 (next point release) milestone Dec 2, 2016

anntzer force-pushed the cleanup branch from 44d640b to 1b1393b Compare December 2, 2016 06:44

NelleV changed the title ~~[MRG+1] Cleanups~~ [MRG] Cleanups Dec 2, 2016

NelleV changed the title ~~[MRG] Cleanups~~ [MRG+3] Cleanups Dec 2, 2016

anntzer force-pushed the cleanup branch from 1b1393b to f6bbc24 Compare December 3, 2016 03:59

anntzer force-pushed the cleanup branch from f6bbc24 to afa9344 Compare December 3, 2016 05:09

tacaswell closed this Dec 3, 2016

tacaswell reopened this Dec 3, 2016

QuLogic reviewed Dec 3, 2016

View reviewed changes

Cleanup some bare excepts and related code.

de5a4ee

anntzer added 3 commits December 3, 2016 14:38

Don't go via six for callable(); related cleanups.

cdb830d

np.pi / np.deg2rad / np.rad2deg where appropriate.

69702f3

Remove nonworking ArtistInspector.findobj.

7722070

anntzer force-pushed the cleanup branch from afa9344 to 7722070 Compare December 3, 2016 22:38

QuLogic changed the title ~~[MRG+3] Cleanups~~ Cleanups Dec 4, 2016

QuLogic merged commit 5b1d671 into matplotlib:master Dec 4, 2016

anntzer deleted the cleanup branch December 4, 2016 23:19

tacaswell mentioned this pull request Dec 22, 2016

BUG: Can't import matplotlib._backports #7663

Closed

4 tasks

		import numpy as np


		# Copy-pasted from numpy.lib.stride_tricks 1.11.2.

Uh oh!

Conversation

anntzer commented Dec 1, 2016

Uh oh!

NelleV commented Dec 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dopplershift left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Dec 3, 2016

Uh oh!

QuLogic commented Dec 3, 2016

Uh oh!

anntzer commented Dec 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anntzer commented Dec 3, 2016

Uh oh!

QuLogic commented Dec 3, 2016

anntzer commented Dec 3, 2016 •

edited

Loading

codecov-io commented Dec 3, 2016 •

edited

Loading