Warn on redundant definition of plot properties #19277

timhoffm · 2021-01-11T20:12:37Z

plt.plot(x, y, fmt) allows to specify marker, linestyle and color via
fmt. Warn if there are additionally keyword arguments that specify the
same properties.

Closes #19275.

Builds on top of #19278, which cleans up _plot_args(). This separation
is done for easier review of the cleanup.

jklymak · 2021-01-11T20:33:14Z

lib/matplotlib/axes/_base.py

-                kw[k] = v
+                if prop_name in kwargs:
+                    _api.warn_external(
+                        f"'{prop_name}' is redundantly defined via fmt string "


A lot of users are not going to know what fmt refers to, particularly those who will put the information in two places. Can we be more explicit here?

linestyle is redundantly defined by the "linestyle" keyword argument and the fmt string ("--r"). The keyword argument will take precedence.

I think that requires saving tup[-1], but that isn't too hard.

Done. Took the opportunity to clean up the function (#19277) and not make it even more confusing by saving a bogous tup[-1].

`fmt`. Warn if there are additionally keyword arguments that specify the same properties. Closes matplotlib#19275. Builds on top of matplotlib#19277, which cleans up _plot_args(). This separation is done for easier review of the cleanup.

jklymak

aside from the typo...

lib/matplotlib/axes/_base.py

timhoffm · 2021-01-11T22:48:34Z

Ping @jklymak. I've rewritten since your review:

plot(x, y, 'o', ls='--') would have issued a warning because 'o' implicitly sets the linestyle to 'none'. This caused failures in our test suite. Moreover, I believe that such a pattern might be quite common in the wild. Semantically it's still reasonably clear that you want a 'o' marker and a line. Therefore I don't warn in this case - could be annoying for users otherwise.

jklymak · 2021-01-11T22:51:16Z

Oh, interesting. But your solution seems correct, so my review is still good.

timhoffm · 2021-01-11T23:12:24Z

It's surprisingly difficult to get this right. 🙄

jklymak · 2021-01-12T00:51:10Z

It's surprisingly difficult to get this right. 🙄

Its not the most straightforward API on the planet. I don't know if it helps - but if its a real nuisance to do the extra verbosity I suggested, feel free to remove it. That was just meant to be helpful, not a blocker.

`plt.plot(x, y, fmt)` allows to specify marker, linestyle and color via `fmt`. Warn if there are additionally keyword arguments that specify the same properties. Closes matplotlib#19275.

anntzer · 2021-01-13T09:01:29Z

lib/matplotlib/axes/_base.py

@@ -117,6 +117,9 @@ def __call__(self, ax, renderer):
            self._transform - ax.figure.transSubfigure)


+_FORMAT_UNSET = 'None'


should that not have been an object() placeholder, to avoid confusion with other meanings of the string "none"?

@anntzer When linestyle or marker are not specified in a format string, they are deactivated. "None" actually serves as value down the road in set_linestyle(). We cannot have a proper sentinel (or would have to convert it again down the road).

My intention was to make it more clear what the semantics of the value are. Otherwise

matplotlib/lib/matplotlib/axes/_base.py

Line 474 in 2219872

and val != _FORMAT_UNSET):

would be and val != 'None'. Which is less understandable, and freaks me out because of the case-insensitivity of our 'none' string handling. I was about to write and val.lower() != 'none' since I didn't follow closely where that was coming from, but then again, val can be a 4-tuple for colors, which you cannot lower().

But thinking again, it's better to be explicit with 'none' and not pretend to have a sentinel, that actually has hidden meaning. See #19291.

Edit: It's not that "simple" 🙄 . #19291 fails a number of tests.

thanks for taking care of that.

rr326 · 2021-03-31T20:48:21Z

I don't think this is a 'bug', but the behavior is a bit confusing and I wonder if it's the new implementation or something I'm somehow doing incorrectly.

My code:

line_median = plt.plot_date(median.index, median, label='median',marker=None,
                             linewidth=2, color='#0f3352', alpha=1, linestyle='solid')

Warning:

UserWarning: marker is redundantly defined by the 'marker' keyword argument and the fmt string "o" (-> marker='o'). The keyword argument will take precedence.
  line_median = plt.plot_date(median.index, median, label='median',marker="",

I can't for the life of me figure out what I'm doing wrong. I don't have a fmt string, I'm specifying individual keywords. It's doing the correct thing, but I don't know what is triggering the warning.

QuLogic · 2021-03-31T20:53:03Z

plot_date has a non-default fmt='o' in its signature, unlike plot. Are you sure you actually need plot_date, and can't use plot directly?

rr326 · 2021-03-31T20:55:20Z

Is there a way to essentially disable that fmt string? I tried None, and "", and neither worked cleanly. It's not the end of the but I'd like to figure out how to run my code without confusing warnings. (And thanks.)

jklymak · 2021-03-31T20:55:59Z

Usually the simple solution is don't use plot_date.

rr326 · 2021-03-31T20:56:02Z

Oh - I see what you wrote - Use plot directly. I can try that. Thanks.

timhoffm added the API: argument checking label Jan 11, 2021

timhoffm added this to the v3.4.0 milestone Jan 11, 2021

jklymak reviewed Jan 11, 2021

View reviewed changes

timhoffm force-pushed the warn-plot-fmt-and-kwarg branch from 07f7a83 to 1eb540a Compare January 11, 2021 21:52

timhoffm marked this pull request as draft January 11, 2021 21:53

timhoffm force-pushed the warn-plot-fmt-and-kwarg branch from 1eb540a to a32934c Compare January 11, 2021 22:19

timhoffm added the status: waiting for other PR label Jan 11, 2021

jklymak approved these changes Jan 11, 2021

View reviewed changes

lib/matplotlib/axes/_base.py Outdated Show resolved Hide resolved

timhoffm force-pushed the warn-plot-fmt-and-kwarg branch 2 times, most recently from b8edf33 to 64eca18 Compare January 11, 2021 22:45

timhoffm marked this pull request as ready for review January 11, 2021 22:48

timhoffm force-pushed the warn-plot-fmt-and-kwarg branch from 64eca18 to cada941 Compare January 11, 2021 23:11

timhoffm force-pushed the warn-plot-fmt-and-kwarg branch 2 times, most recently from 58dc06c to 12f6a9e Compare January 11, 2021 23:57

timhoffm force-pushed the warn-plot-fmt-and-kwarg branch from 12f6a9e to bf7449f Compare January 12, 2021 10:38

Warn on redundant definition of plot properties

c000643

`plt.plot(x, y, fmt)` allows to specify marker, linestyle and color via `fmt`. Warn if there are additionally keyword arguments that specify the same properties. Closes matplotlib#19275.

timhoffm force-pushed the warn-plot-fmt-and-kwarg branch from bf7449f to c000643 Compare January 13, 2021 00:26

timhoffm removed the status: waiting for other PR label Jan 13, 2021

QuLogic approved these changes Jan 13, 2021

View reviewed changes

QuLogic merged commit 2219872 into matplotlib:master Jan 13, 2021

anntzer reviewed Jan 13, 2021

View reviewed changes

timhoffm mentioned this pull request Jan 14, 2021

Cleanup code for format processing #19291

Merged

timhoffm deleted the warn-plot-fmt-and-kwarg branch January 14, 2021 00:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn on redundant definition of plot properties #19277

Warn on redundant definition of plot properties #19277

timhoffm commented Jan 11, 2021 •

edited

Loading

jklymak Jan 11, 2021

timhoffm Jan 11, 2021

jklymak Jan 11, 2021

jklymak left a comment

timhoffm commented Jan 11, 2021

jklymak commented Jan 11, 2021

timhoffm commented Jan 11, 2021

jklymak commented Jan 12, 2021

anntzer Jan 13, 2021

timhoffm Jan 14, 2021 •

edited

Loading

anntzer Jan 14, 2021

rr326 commented Mar 31, 2021

QuLogic commented Mar 31, 2021 •

edited

Loading

rr326 commented Mar 31, 2021

jklymak commented Mar 31, 2021

rr326 commented Mar 31, 2021

		@@ -117,6 +117,9 @@ def __call__(self, ax, renderer):
		self._transform - ax.figure.transSubfigure)


		_FORMAT_UNSET = 'None'

Warn on redundant definition of plot properties #19277

Warn on redundant definition of plot properties #19277

Conversation

timhoffm commented Jan 11, 2021 • edited Loading

jklymak Jan 11, 2021

Choose a reason for hiding this comment

timhoffm Jan 11, 2021

Choose a reason for hiding this comment

jklymak Jan 11, 2021

Choose a reason for hiding this comment

jklymak left a comment

Choose a reason for hiding this comment

timhoffm commented Jan 11, 2021

jklymak commented Jan 11, 2021

timhoffm commented Jan 11, 2021

jklymak commented Jan 12, 2021

anntzer Jan 13, 2021

Choose a reason for hiding this comment

timhoffm Jan 14, 2021 • edited Loading

Choose a reason for hiding this comment

anntzer Jan 14, 2021

Choose a reason for hiding this comment

rr326 commented Mar 31, 2021

QuLogic commented Mar 31, 2021 • edited Loading

rr326 commented Mar 31, 2021

jklymak commented Mar 31, 2021

rr326 commented Mar 31, 2021

timhoffm commented Jan 11, 2021 •

edited

Loading

timhoffm Jan 14, 2021 •

edited

Loading

QuLogic commented Mar 31, 2021 •

edited

Loading