Skip to content

Improve error for invalid format strings / misspelled data keys. #23088

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 1 commit into from
May 22, 2022
Merged

Improve error for invalid format strings / misspelled data keys. #23088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 22 additions & 14 deletions lib/matplotlib/axes/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def __call__(self, ax, renderer):
self._transform - ax.figure.transSubfigure)


def _process_plot_format(fmt):
def _process_plot_format(fmt, *, ambiguous_fmt_datakey=False):
Copy link
Member

Choose a reason for hiding this comment

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

Bonus points if you copy the docstring for the parameter here as well.

Copy link
Contributor Author

@anntzer anntzer May 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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.

"""
Convert a MATLAB style color/line style format string to a (*linestyle*,
*marker*, *color*) tuple.
Expand Down Expand Up @@ -163,31 +163,31 @@ def _process_plot_format(fmt):
except ValueError:
pass # No, not just a color.

errfmt = ("{!r} is neither a data key nor a valid format string ({})"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
errfmt = ("{!r} is neither a data key nor a valid format string ({})"
errfmt = ("{!r} is not a data key or a valid format string ({})"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do prefer neither/nor, so I'll go back to that :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. :) I'm not formal enough in my speaking it seems!

if ambiguous_fmt_datakey else
"{!r} is not a valid format string ({})")

i = 0
while i < len(fmt):
c = fmt[i]
if fmt[i:i+2] in mlines.lineStyles: # First, the two-char styles.
if linestyle is not None:
raise ValueError(
f'Illegal format string {fmt!r}; two linestyle symbols')
raise ValueError(errfmt.format(fmt, "two linestyle symbols"))
linestyle = fmt[i:i+2]
i += 2
elif c in mlines.lineStyles:
if linestyle is not None:
raise ValueError(
f'Illegal format string {fmt!r}; two linestyle symbols')
raise ValueError(errfmt.format(fmt, "two linestyle symbols"))
linestyle = c
i += 1
elif c in mlines.lineMarkers:
if marker is not None:
raise ValueError(
f'Illegal format string {fmt!r}; two marker symbols')
raise ValueError(errfmt.format(fmt, "two marker symbols"))
marker = c
i += 1
elif c in mcolors.get_named_colors_mapping():
if color is not None:
raise ValueError(
f'Illegal format string {fmt!r}; two color symbols')
raise ValueError(errfmt.format(fmt, "two color symbols"))
color = c
i += 1
elif c == 'C' and i < len(fmt) - 1:
Expand All @@ -196,7 +196,7 @@ def _process_plot_format(fmt):
i += 2
else:
raise ValueError(
f'Unrecognized character {c} in format string {fmt!r}')
errfmt.format(fmt, f"unrecognized character {c!r}"))

if linestyle is None and marker is None:
linestyle = mpl.rcParams['lines.linestyle']
Expand Down Expand Up @@ -293,6 +293,7 @@ def __call__(self, *args, data=None, **kwargs):
kwargs["label"] = mpl._label_from_arg(
replaced[label_namer_idx], args[label_namer_idx])
args = replaced
ambiguous_fmt_datakey = data is not None and len(args) == 2

if len(args) >= 4 and not cbook.is_scalar_or_string(
kwargs.get("label")):
Expand All @@ -308,7 +309,8 @@ def __call__(self, *args, data=None, **kwargs):
if args and isinstance(args[0], str):
this += args[0],
args = args[1:]
yield from self._plot_args(this, kwargs)
yield from self._plot_args(
this, kwargs, ambiguous_fmt_datakey=ambiguous_fmt_datakey)

def get_next_color(self):
"""Return the next color in the cycle."""
Expand Down Expand Up @@ -402,7 +404,8 @@ def _makefill(self, x, y, kw, kwargs):
seg.set(**kwargs)
return seg, kwargs

def _plot_args(self, tup, kwargs, return_kwargs=False):
def _plot_args(self, tup, kwargs, *,
return_kwargs=False, ambiguous_fmt_datakey=False):
"""
Process the arguments of ``plot([x], y, [fmt], **kwargs)`` calls.

Expand All @@ -429,9 +432,13 @@ def _plot_args(self, tup, kwargs, return_kwargs=False):
The keyword arguments passed to ``plot()``.

return_kwargs : bool
If true, return the effective keyword arguments after label
Whether to also return the effective keyword arguments after label
unpacking as well.

ambiguous_fmt_datakey : bool
Whether the format string in *tup* could also have been a
misspelled data key.

Returns
-------
result
Expand All @@ -445,7 +452,8 @@ def _plot_args(self, tup, kwargs, return_kwargs=False):
if len(tup) > 1 and isinstance(tup[-1], str):
# xy is tup with fmt stripped (could still be (y,) only)
*xy, fmt = tup
linestyle, marker, color = _process_plot_format(fmt)
linestyle, marker, color = _process_plot_format(
fmt, ambiguous_fmt_datakey=ambiguous_fmt_datakey)
elif len(tup) == 3:
raise ValueError('third arg must be a format string')
else:
Expand Down
19 changes: 11 additions & 8 deletions lib/matplotlib/tests/test_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7698,16 +7698,19 @@ def test_empty_line_plots():


@pytest.mark.parametrize('fmt, match', (
("foo", "Unrecognized character f in format string 'foo'"),
("o+", "Illegal format string 'o\\+'; two marker symbols"),
(":-", "Illegal format string ':-'; two linestyle symbols"),
("rk", "Illegal format string 'rk'; two color symbols"),
(":o-r", "Illegal format string ':o-r'; two linestyle symbols"),
("f", r"'f' is not a valid format string \(unrecognized character 'f'\)"),
("o+", r"'o\+' is not a valid format string \(two marker symbols\)"),
(":-", r"':-' is not a valid format string \(two linestyle symbols\)"),
("rk", r"'rk' is not a valid format string \(two color symbols\)"),
(":o-r", r"':o-r' is not a valid format string \(two linestyle symbols\)"),
))
def test_plot_format_errors(fmt, match):
@pytest.mark.parametrize("data", [None, {"string": range(3)}])
def test_plot_format_errors(fmt, match, data):
fig, ax = plt.subplots()
with pytest.raises(ValueError, match=match):
ax.plot((0, 0), fmt)
if data is not None:
match = match.replace("not", "neither a data key nor")
with pytest.raises(ValueError, match=r"\A" + match + r"\Z"):
ax.plot("string", fmt, data=data)


def test_clim():
Expand Down