Clarifying an error message #11772
Clarifying an error message #11772
Conversation
|
Thanks for the contribution! (as an aside, I recommend "Think Python" to everyone) Can you please provide sample code that triggered this error? I'm having trouble figuring out which argument was the format string. |
|
Sample code: |
| @@ -350,7 +350,7 @@ def _plot_args(self, tup, kwargs): | |||
| # to one element array of None which causes problems | |||
| # downstream. | |||
| if any(v is None for v in tup): | |||
| raise ValueError("x and y must not be None") | |||
| raise ValueError("x, y, and format string must not be None") | |||
There was a problem hiding this comment.
Huh, you shouldn't have got here according to the logic above.... I think you've actually found a bug, not a documentation error...
|
If the function can gracefully handle `fmt=None`, that would be even better.
…On Wed, Jul 25, 2018 at 11:04 AM, Jody Klymak ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Thanks for the PR, but I don't think you should have got to this Error if
the third positional argument was not a string. So, I'm blocking because I
think we should fix the actual bug...
------------------------------
In lib/matplotlib/axes/_base.py
<#11772 (comment)>
:
> @@ -350,7 +350,7 @@ def _plot_args(self, tup, kwargs):
# to one element array of None which causes problems
# downstream.
if any(v is None for v in tup):
- raise ValueError("x and y must not be None")
+ raise ValueError("x, y, and format string must not be None")
Huh, you shouldn't have got here according to the logic above.... I think
you've actually found a bug, not a documentation error...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#11772 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABy37W1K9hNb-3Z9vADZ_2CfHwoPCA0aks5uKIkHgaJpZM4VgLfA>
.
|
|
It's not clear to me why we should accept None (an empty string is even shorter to type), so I agree with the original PR. @jklymak Not sure what is the logic error you're seeing... |
|
Well, `fmt=None` came up naturally in my use case, which involves parsing
arguments and passing the result along to `plt.plot`. If `plt.plot`
doesn't accept `fmt=None`, I have to special case it.
…On Wed, Jul 25, 2018 at 11:09 AM, Antony Lee ***@***.***> wrote:
It's not clear to me why we should accept None (an empty string is even
shorter to type), so I agree with the original PR. @jklymak
<https://github.com/jklymak> Not sure what is the logic error you're
seeing...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#11772 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABy37beeMvr9pso-hX0Rt6yYsPZ1Ihocks5uKIojgaJpZM4VgLfA>
.
|
|
The logic in However, _plot_args is not being passed a three-tuple in this case. It gets passed a 2-tuple (x and y) and then a 1-tuple (None). That seems a bit strange to me... |
|
@AllenDowney But certainly you can make the parsing parse whatever input into "" instead of None? @jklymak That's because we're treating (or trying to treat) this as we do for i.e. plot [1, 2, 3] against [4, 5, 6] and (implicit index of [0, 1, 2]) against [7, 8, 9]. Is that a good API? Probably not (looks more likely to hide logic bugs). |
|
Fair enough. Boy is the |
|
Ah but I guess should be considered as "fine" (as in, likely not a logic bug) so heh... |
|
Thanks @AllenDowney |
|
Thank you all. |
PR Summary
If the format string is None, you get a message that says "x and y must not be None", which is confusing.
I'm suggesting a revised error message.
PR Checklist