Mnt/generalize plot varargs #28653
Mnt/generalize plot varargs #28653
Conversation
lib/matplotlib/axes/_base.py
Outdated
| @@ -214,6 +214,7 @@ class _process_plot_var_args: | |||
| """ | |||
|
|
|||
| def __init__(self, command='plot'): | |||
| _api.check_in_list(['plot', 'fill', 'mirror'], command=command) | |||
There was a problem hiding this comment.
The command API was not great, not documented, but sort of understandable:
The command is the Axes method that calls the class ax.plot() / ax.fill(). The created artists are the return types of that function. Current nuissance: To be more clear the generator functions should be called _make_Line2D and _make_Polygon (modulo capitalization).
I don't understand what "mirror" is for.
| command | "plot" | "fill" | "mirror" |
|---|---|---|---|
| created artist | Line2D | Polygon | ? |
| internal generator function | _makeline | _makefill | _mirror |
There was a problem hiding this comment.
We wanted to be able to re-use all of the machinery for processing the vargs, but get the per-artist args/wkwargs out so the we can create new-style artist from them (which we can not inject into core Matplotlib yet).
I agree it is not great, but was going for the minimal change that would unblock the data prototype work without having to re-write this whole API!
Instead of "mirror" use "no_artist" instead?
There was a problem hiding this comment.
This is all internal, so renaming would be possible. However there's a slight complication that command is used in raise _api.kwarg_error(self.command, pos_only). With that we inject the usage context name into the class, so that when called from plot() it can issue an error containing "plot" and likewise for "fill". This means that _process_plot_var_args is not really context free and you'd have to hard-code in which places you want to use it to support a proper exception.
The "cheap" solution would be command="UNKNOWN" or similar for the new case, which would result in
UNKNOWN() got an unexpected keyword argument 'x'
That's sort of ok.
The IMHO slightly better solution would be to decouple the function from the caller, i.e. return a generic "unexpected keyword arguments 'x'" and - if desired - catch and ammend that in the caller. This would give the freedom to rename command - e.g. to artist with values Line2D, Polygon and None.
|
@tacaswell I've been so bold to push a cleanup commit in the spirit of #28653 (comment)
IMHO this makes the sematics much more clear. Please have a look if you agree. If you don't agree, feel free to modify or drop that commit. |
Distinguish variants by desired output instead of calling command. This gives a better terminology for the new coordinate output variant. The command input was also used to create an error message containing the calling function name. This has been replaced by introspection.
603d823 to
ca2ecb2
Compare
| axes._process_unit_info(kwargs=kwargs) | ||
|
|
||
| for pos_only in "xy": | ||
| if pos_only in kwargs: | ||
| raise _api.kwarg_error(self.command, pos_only) | ||
| raise _api.kwarg_error(inspect.stack()[1].function, pos_only) |
There was a problem hiding this comment.
I'm a bit wary of stack walking, but this is only in an case were we are already going to errer out and raise it is OK.
|
I am 👍🏻 on @timhoffm changes. |
PR summary
This extends the
_plot_var_argshelper class slightly so that it can be used in the data-prototype work. The key idea here is that in addition to being able to return instantiated Artists, it can also return the inferred x, y, and kwargs.This is all private API.
PR checklist