Add deprecation messages for unused kwargs in FancyArrowPatch #11224
Conversation
lib/matplotlib/patches.py
Outdated
| message=('The "connector" keyword argument is not used,' | ||
| ' and will be removed in Matplotlib 3.1'), | ||
| name='arrow_transmuter', | ||
| obj_type='keyword argument') |
There was a problem hiding this comment.
should be name=connector' ?
Fix variable name
7e82f3f to
2cdb484
Compare
| @@ -100,6 +100,7 @@ def warn_deprecated( | |||
| """ | |||
| message = _generate_deprecation_message( | |||
| since, message, name, alternative, pending, obj_type, removal=removal) | |||
| message = '\n' + message | |||
There was a problem hiding this comment.
Minor style thing: I would not do this explicitly on a separate line, but rather:
warnings.warn('\n' + message, ...)
There was a problem hiding this comment.
I'm actually quite strongly -1 on embedding explicit newlines in warnings and error messages.
There was a problem hiding this comment.
Because you are making assumptions about the width of the window of the output (most of the time a normal linewrap will just work as well); moreover people can for example capture warnings into the logging system (logging.captureWarnings) and do all kinds of custom formatting of the message there as well.
Not going to block the PR over that, just giving my opinion though.
There was a problem hiding this comment.
I think of it as making assumptions about the maximum number of characters that should reasonably fit on one line, with the linebreak making the message as a whole more readable. I'd be inclined to say that if advanced users want to do custom formatting then it's much easier to remove line breaks than try and work out where to put them in.
| cbook.warn_deprecated( | ||
| 3.0, | ||
| message=('The "arrow_transmuter" keyword argument is not used,' | ||
| ' and will be removed in Matplotlib 3.1'), |
There was a problem hiding this comment.
You can do something like cbook.warn_deprecated("3.0", "arrow_transmuter", obj_type="keyword argument", removal="3.1") and that'll give you the right message.
There was a problem hiding this comment.
@dstansby, did you want to do this, or should I just merge as is?
There was a problem hiding this comment.
If the message looks fine as is then can it be merged as is?
|
@timhoffm @tacaswell - Can this be reverted? I just ventured here after looking for a reason why my warnings filter stopped working. # before
filterwarnings('ignore', message='Adding an axes using the same arguments')
# now
filterwarnings('ignore', message=r'\s*Adding an axes using the same arguments') |
This breaks the summary from Pytest when warnings are errors, as then you just get a bunch of: ``` FAILED lib/matplotlib/tests/test_getattr.py::test_getattr[matplotlib.afm] - matplotlib._api.deprecation.MatplotlibDeprecationWarning: ``` with no real context as to the problem. Also, as noted at the original PR [1], it makes setting `filterwarnings` more complicated. [1] matplotlib#11224 (comment)
This breaks the summary from Pytest when warnings are errors, as then you just get a bunch of: ``` FAILED lib/matplotlib/tests/test_getattr.py::test_getattr[matplotlib.afm] - matplotlib._api.deprecation.MatplotlibDeprecationWarning: ``` with no real context as to the problem. Also, as noted at the original PR [1], it makes setting `filterwarnings` more complicated. [1] matplotlib#11224 (comment)
I've gone with removal in
3.1, but I'm not sure if that's too soon or not? I've also added a newline before deprecation messages, as it makes them more readable, changing them from:to