Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Warn on redundant definition of plot properties#19277
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
lib/matplotlib/axes/_base.py Outdated
| kw[k] = v | ||
| if prop_name in kwargs: | ||
| _api.warn_external( | ||
| f"'{prop_name}' is redundantly defined via fmt string " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A lot of users are not going to know whatfmt 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 savingtup[-1], but that isn't too hard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done. Took the opportunity to clean up the function (#19277) and not make it even more confusing by saving a bogoustup[-1].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
#19278...
07f7a83 to1eb540aCompare`fmt`. Warn if there are additionally keyword arguments that specify thesame properties.Closesmatplotlib#19275.Builds on top ofmatplotlib#19277, which cleans up _plot_args(). This separationis done for easier review of the cleanup.
1eb540a toa32934cCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
aside from the typo...
Uh oh!
There was an error while loading.Please reload this page.
b8edf33 to64eca18ComparePing@jklymak. I've rewritten since your review:
|
Oh, interesting. But your solution seems correct, so my review is still good. |
64eca18 tocada941CompareIt's surprisingly difficult to get this right. 🙄 |
58dc06c to12f6a9eCompare
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. |
12f6a9e tobf7449fCompare`plt.plot(x, y, fmt)` allows to specify marker, linestyle and color via`fmt`. Warn if there are additionally keyword arguments that specify thesame properties.Closesmatplotlib#19275.
bf7449f toc000643Compare| @@ -117,6 +117,9 @@ def __call__(self, ax, renderer): | |||
| self._transform - ax.figure.transSubfigure) | |||
| _FORMAT_UNSET = 'None' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should that not have been anobject() placeholder, to avoid confusion with other meanings of the string "none"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@anntzer When linestyle or marker are not specified in a format string, they are deactivated. "None" actually serves as value down the road inset_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 in2219872
| andval!=_FORMAT_UNSET): |
would beand val != 'None'. Which is less understandable, and freaks me out because of the case-insensitivity of our 'none' string handling. I was about to writeand 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 cannotlower().
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
thanks for taking care of that.
rr326 commentedMar 31, 2021
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: 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 commentedMar 31, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
rr326 commentedMar 31, 2021
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.) |
Usually the simple solution is don't use plot_date. |
rr326 commentedMar 31, 2021
Oh - I see what you wrote - Use plot directly. I can try that. Thanks. |
Uh oh!
There was an error while loading.Please reload this page.
plt.plot(x, y, fmt)allows to specify marker, linestyle and color viafmt. Warn if there are additionally keyword arguments that specify thesame properties.
Closes#19275.
Builds on top of#19278, which cleans up _plot_args(). This separation
is done for easier review of the cleanup.