Fix a number of Deprecated/Invalid escape sequences #7925

Carreau · 2017-01-23T00:21:43Z

And turn python 3.6 DeprecationWarnings into error during tests.

Python 3.6 deprecate invalid escape sequences, as they are (most of the
time) mistakes from the author. This introduce extra \ when necessary,
or make strings raw.

See #7924

QuLogic · 2017-01-23T00:43:37Z

lib/matplotlib/backends/backend_ps.py

@@ -5,6 +5,7 @@
 from __future__ import (absolute_import, division, print_function,
                        unicode_literals)

+


Unnecessary change.

Sorry, autopep8 script doing more than asked.

QuLogic · 2017-01-23T00:43:48Z

lib/matplotlib/backends/backend_ps.py

@@ -1543,7 +1544,6 @@ def gs_distill(tmpfile, eps=False, ptype='letter', bbox=None, rotated=False):
        else:
            pstoeps(tmpfile)

-


Should not be changed.

QuLogic · 2017-01-23T00:44:49Z

lib/matplotlib/font_manager.py

@@ -184,7 +184,7 @@ def win32FontDirectory():
    Return the user-specified font directory for Win32.  This is
    looked up from the registry key::

-      \\HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Explorer\Shell Folders\Fonts
+      \\HKEY_CURRENT_USER\\Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Shell Folders\\Fonts


The first one is supposed to be doubled, so it should be 4 when escaped, but maybe it'd be simpler to make it a raw string instead?

Oh.. ok, I did not remembered \\ was a thing on windows.

QuLogic · 2017-01-23T00:46:07Z

lib/matplotlib/mathtext.py

-                          ('\rightbrace', '}'),
-                          ('\leftbracket', '['),
-                          ('\rightbracket', ']'),
+    for alias, target in [('\\leftparen', '('),


The last ones are raw strings; better to do that for consistency.

sure. Done.

QuLogic · 2017-01-23T00:48:00Z

lib/matplotlib/patches.py

@@ -1803,7 +1803,7 @@ def _pprint_styles(_styles):
    (stylename : styleclass), return a formatted string listing all the
    styles. Used to update the documentation.
    """
-    names, attrss, clss = [], [], []
+    ddnames, attrss, clss = [], [], []


Hum.. vim, probably though I was typing in another window.

though these 3 attributes are not use at all in the function...

QuLogic · 2017-01-23T00:50:55Z

lib/matplotlib/tests/test_subplots.py

@@ -111,7 +111,7 @@ def test_exceptions():
    # the point of this test is to ensure that this raises.
    with warnings.catch_warnings():
        warnings.filterwarnings('ignore',
-                                message='.*sharex\ argument\ to\ subplots',
+                                message=r'.*sharex\ argument\ to\ subplots',


What is \ ? I don't think this escape even means anything in a regular expression.

QuLogic · 2017-01-23T00:51:48Z

lib/matplotlib/tests/test_text.py

@@ -30,7 +30,7 @@ def find_matplotlib_font(**kw):
    from matplotlib.font_manager import FontProperties, findfont
    warnings.filterwarnings(
        'ignore',
-        ('findfont: Font family \[u?\'Foo\'\] not found. Falling back to .'),
+        r'findfont: Font family \[u?\'Foo\'\] not found. Falling back to .',


I think the internal quotes no longer need to be escaped here now.

Use double quotes instead?

Err, actually, I don't know what I was thinking here, but the internal quotes still need to be escaped unless using double quotes as @anntzer suggested.

@Carreau As mentioned just above, can you switch to double quotes and remove the backslashes in the inner single quotes here?

QuLogic · 2017-01-23T00:53:15Z

tests.py

+
+    # Python 3.6 deprecate invalid character-pairs \A, \* ... in non raw-strings
+    # and other things. Let's not re-introduce them
+    warnings.filterwarnings('error', category=DeprecationWarning)


Though I prefer not to introduce any deprecations either, this seems a bit broad for this PR.

codecov-io · 2017-01-23T02:25:20Z

Current coverage is 62.18% (diff: 100%)

Merging #7925 into master will not change coverage

@@             master      #7925   diff @@
==========================================
  Files           175        175          
  Lines         56125      56125          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          34901      34901          
  Misses        21224      21224          
  Partials          0          0

Powered by Codecov. Last update c93142f...582d771

anntzer · 2017-01-23T08:48:53Z

tests.py

+
+    # The warnings need to be before any of matplotlib imports, but after
+    # setuptools (if present) which has syntax error with the warnings enabled.
+    # Filtering by module does not work as this will be raise by Python itself.


typo: raised

Carreau · 2017-01-25T00:21:36Z

Rebased to remove conflicts.

Carreau · 2017-01-27T23:07:51Z

Rebased again and use backtick

NelleV

LGTM 👍
Thanks @Carreau

jkseppan · 2017-01-31T17:58:02Z

tests.py

+        'default',
+        '.*inspect.getargspec\(\) is deprecated.*',
+        category=DeprecationWarning)
+


This only gets executed when running tests.py, and I think most tests have been migrated to pytest now. Is there a way to make pytest execute these filterwarnings calls?

Yes. It's supposed to get folded into Pytest itself at some point.

tacaswell · 2017-01-31T19:03:52Z

lib/matplotlib/collections.py

@@ -394,7 +394,7 @@ def set_hatch(self, hatch):
        *hatch* can be one of::

          /   - diagonal hatching
-          \   - back diagonal
+          \\   - back diagonal


This one worries me as it is something I would expect to be read either on the web or in the terminal. Probably better to make this a r''' string?

and I may be wrong here, escaping always ties me in knots....

Done. I can't rebuild matplotlib today apparently, so can't test the html versions.

Actually I managed to build the docs, and the \ in Accept below needed to be escaped more.
That should be fixed.

And turn python 3.6 DeprecationWarnings into error during tests. Python 3.6 deprecate invalid escape sequences, as they are (most of the time) mistakes from the author. This introduce extra `\` when necessary, or make strings `raw`. See matplotlib#7924

QuLogic · 2017-02-01T00:16:04Z

lib/matplotlib/collections.py

@@ -415,7 +415,7 @@ def set_hatch(self, hatch):
        can only be specified for the collection as a whole, not separately
        for each member.

-        ACCEPTS: [ '/' | '\\\\' | '|' | '-' | '+' | 'x' | 'o' | 'O' | '.' | '*' ]
+        ACCEPTS: [ '/' | '\\' | '|' | '-' | '+' | 'x' | 'o' | 'O' | '.' | '*' ]


It appears that this line is under 79 characters and this file now passes PEP8, so it needs to be removed from lib/matplotlib/tests/test_coding_standards.py because the build is failing.

Also, since you're going to have to push an update, I cancelled the OSX build which has been waiting for a while because we've got a long backlog.

anntzer

Minor nitpick.

anntzer · 2017-02-01T07:28:58Z

lib/matplotlib/tests/test_text.py

@@ -30,7 +30,7 @@ def find_matplotlib_font(**kw):
    from matplotlib.font_manager import FontProperties, findfont
    warnings.filterwarnings(
        'ignore',
-        ('findfont: Font family \[u?\'Foo\'\] not found. Falling back to .'),
+        r'findfont: Font family \[u?\'Foo\'\] not found. Falling back to .',


@Carreau As mentioned just above, can you switch to double quotes and remove the backslashes in the inner single quotes here?

NelleV · 2017-02-02T00:31:34Z

Thanks @Carreau
4 core dev validating your PR: I believe this is a new record :)

Carreau · 2017-02-02T00:41:42Z

Thanks !

…

On Wed, Feb 1, 2017 at 4:31 PM, Nelle Varoquaux ***@***.***> wrote: Merged #7925 <#7925>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7925 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUez9dEJtH0NYiDiahyH0-bRrjJIZd8ks5rYSP1gaJpZM4Lqj9-> .

QuLogic reviewed Jan 23, 2017

View reviewed changes

Carreau force-pushed the deprecations branch from 582d771 to 0da46b4 Compare January 23, 2017 02:24

anntzer reviewed Jan 23, 2017

View reviewed changes

Carreau force-pushed the deprecations branch 2 times, most recently from c24f837 to ff6afaf Compare January 25, 2017 00:21

Carreau force-pushed the deprecations branch from ff6afaf to ff4560c Compare January 27, 2017 23:02

NelleV approved these changes Jan 29, 2017

View reviewed changes

NelleV changed the title ~~Fix a number of Deprecated/Invalid escape sequences~~ [MRG+1] Fix a number of Deprecated/Invalid escape sequences Jan 29, 2017

jkseppan reviewed Jan 31, 2017

View reviewed changes

tacaswell reviewed Jan 31, 2017

View reviewed changes

Carreau force-pushed the deprecations branch from ff4560c to 6598d89 Compare January 31, 2017 19:19

tacaswell approved these changes Jan 31, 2017

View reviewed changes

Carreau force-pushed the deprecations branch from 6598d89 to 9212e59 Compare January 31, 2017 20:38

QuLogic reviewed Feb 1, 2017

View reviewed changes

QuLogic approved these changes Feb 1, 2017

View reviewed changes

anntzer requested changes Feb 1, 2017

View reviewed changes

remove collections from expect_bad_files (pep8)

efd5384

Carreau force-pushed the deprecations branch from 204f4de to efd5384 Compare February 1, 2017 17:44

anntzer approved these changes Feb 1, 2017

View reviewed changes

NelleV merged commit 924d913 into matplotlib:master Feb 2, 2017

QuLogic changed the title ~~[MRG+1] Fix a number of Deprecated/Invalid escape sequences~~ Fix a number of Deprecated/Invalid escape sequences Feb 2, 2017

QuLogic added this to the 2.1 (next point release) milestone Feb 2, 2017

anntzer mentioned this pull request Apr 15, 2017

Python 3.6 deprecated escape sequences. #7924

Closed

		@@ -5,6 +5,7 @@
		from __future__ import (absolute_import, division, print_function,
		unicode_literals)

		@@ -1543,7 +1544,6 @@ def gs_distill(tmpfile, eps=False, ptype='letter', bbox=None, rotated=False):
		else:
		pstoeps(tmpfile)

Uh oh!

Fix a number of Deprecated/Invalid escape sequences #7925

Fix a number of Deprecated/Invalid escape sequences #7925

Uh oh!

Conversation

Carreau commented Jan 23, 2017

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!

codecov-io commented Jan 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 62.18% (diff: 100%)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Carreau commented Jan 25, 2017

Uh oh!

Carreau commented Jan 27, 2017

Uh oh!

NelleV 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!

anntzer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NelleV commented Feb 2, 2017

Uh oh!

Carreau commented Feb 2, 2017 via email

Uh oh!

Uh oh!

codecov-io commented Jan 23, 2017 •

edited

Loading