Improve error for invalid format strings / misspelled data keys.#23088
Improve error for invalid format strings / misspelled data keys.#23088jklymak merged 1 commit intomatplotlib:mainfrom
Conversation
bd952fb to
d7bf480
Compare
|
|
||
|
|
||
| def _process_plot_format(fmt): | ||
| def _process_plot_format(fmt, *, ambiguous_fmt_datakey=False): |
There was a problem hiding this comment.
Bonus points if you copy the docstring for the parameter here as well.
There was a problem hiding this comment.
The docstring was already not numpydoc'ed (and doing properly so is slightly messy due to, well, the messy API), so I'll skip these points for now.
greglucas
left a comment
There was a problem hiding this comment.
Minor wording suggestion to avoid the double negative, but it looks good to me.
| except ValueError: | ||
| pass # No, not just a color. | ||
|
|
||
| errfmt = ("{!r} is neither a data key nor a valid format string ({})" |
There was a problem hiding this comment.
| errfmt = ("{!r} is neither a data key nor a valid format string ({})" | |
| errfmt = ("{!r} is not a data key or a valid format string ({})" |
There was a problem hiding this comment.
This is a bit of an Oxford comma thing, but I would tend to say neither...nor here. It's correct and a bit more formal.
There was a problem hiding this comment.
I do prefer neither/nor, so I'll go back to that :)
There was a problem hiding this comment.
Sounds good. :) I'm not formal enough in my speaking it seems!
b40e6dd to
d7bf480
Compare
|
I'll take the liberty of merging because I don't think @greglucas was that worried about the grammar change ;-) |
PR Summary
Closes #23083 (IMO).
FWIW I agree that the current behavior can be a bit confusing (I'm not sure really like the whole data kwarg business due to the confusion it creates in combination with plot()'s weird signatures, but that ship has sailed long ago).
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).