8000 Add deprecation messages for unused kwargs in FancyArrowPatch by dstansby · Pull Request #11224 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Add deprecation messages for unused kwargs in FancyArrowPatch #11224

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

Merged
merged 2 commits into from
Jun 16, 2018
Merged

Add deprecation messages for unused kwargs in FancyArrowPatch #11224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51e4944
Add a newline before deprecation messages
dstansby May 10, 2018
2cdb484
Add deprecation messages for unused kwargs in FancyArrowPatch
dstansby May 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/matplotlib/cbook/deprecation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def warn_deprecated(
"""
message = _generate_deprecation_message(
since, message, name, alternative, pending, obj_type, removal=removal)
message = '\n' + message
Copy link
Member

Choose a reason for hiding this comment

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

Minor style thing: I would not do this explicitly on a separate line, but rather:
warnings.warn('\n' + message, ...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be display 10000 ed to describe this comment to others. Learn more.

I'm actually quite strongly -1 on embedding explicit newlines in warnings and error messages.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why, out of interest?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

warnings.warn(message, mplDeprecation, stacklevel=2)


Expand Down
14 changes: 14 additions & 0 deletions lib/matplotlib/patches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4060,6 +4060,20 @@ def __init__(self, posA=None, posB=None,
Valid kwargs are:
%(Patch)s
"""
if arrow_transmuter is not None:
cbook.warn_deprecated(
3.0,
message=('The "arrow_transmuter" keyword argument is not used,'
' and will be removed in Matplotlib 3.1'),
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

@dstansby, did you want to do this, or should I just merge as is?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the message looks fine as is then can it be merged as is?

name='arrow_transmuter',
obj_type='keyword argument')
if connector is not None:
cbook.warn_deprecated(
3.0,
message=('The "connector" keyword argument is not used,'
' and will be removed in Matplotlib 3.1'),
name='connector',
obj_type='keyword argument')
Patch.__init__(self, **kwargs)

if posA is not None and posB is not None and path is None:
Expand Down
0