-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Improve plot() docstring. #12772
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve plot() docstring. #12772
Conversation
lib/matplotlib/axes/_axes.py
Outdated
@@ -1381,8 +1381,8 @@ def plot(self, *args, scalex=True, scaley=True, **kwargs): | |||
|
|||
Call signatures:: | |||
|
|||
plot([x], y, [fmt], data=None, **kwargs) | |||
plot([x], y, [fmt], [x2], y2, [fmt2], ..., **kwargs) | |||
plot([x], y, [fmt], /, *, data=None, **kwargs) |
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.
I'm -1 to this change, because I've been using/developing Matplotlib for a while and don't know what , /, *,
means, so I think this change would make the docstring much less accessible. (I actually think enumerating possible calls is the best way to go here)
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.
I have no idea what that means either. I guess using a short form like this might be OK if the usable;e signatures are explained in detail elsewhere in the doctoring.
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.
There are markers for "positional only before here" and "keyword only after here".
I agree that these are probably not widely known to end-users.
I have on problem leaving these out. What might be a helpful specification for more advanced users can indeed be confusing for less experienced users. Anyway, if someone uses the positional or keyword args incorrectly, they'll get a respective error message.
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.
*
is standard (valid) Py3 syntax to mark that data
is keyword only, so I'm definitely leaving it.
/
is proposed syntax for marking that arguments before it are positional-only (cannot be passed as keywords) (https://www.python.org/dev/peps/pep-0457/#syntax-and-semantics). I'm only putting this in because people have tried (#12106) to pass x
/y
as keyword, and this doesn't work (and I don't think it should, for reasons similar to those discussed in #11526). We are already using this syntax for documenting (the admittedly less commonly used) Axes3D.voxels
(https://matplotlib.org/api/_as_gen/mpl_toolkits.mplot3d.axes3d.Axes3D.html#mpl_toolkits.mplot3d.axes3d.Axes3D.voxels) and Axes3D.quiver
. Similarly to that case, I added a parenthetical note clarifying this in the Parameters section regarding x
and y
.
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.
It's not because it is standard valid py3 syntax that it should be used in our examples :)
940ec25
to
8dcc2e2
Compare
lib/matplotlib/axes/_axes.py
Outdated
|
||
The parameters can also be 2-dimensional. Then, the columns | ||
represent separate data sets. | ||
These arguments cannot be passed as keywords (as indicated by the |
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.
That helps, but fmt can't be passed ask warg either according to the signature?
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.
added a note there too (indeed, it can't)
8dcc2e2
to
fec5dc7
Compare
@dstansby Are you OK with the PR as of now or do you still reject it? |
- Mark `x`, `y`, `fmt` as positional-only. - Scalar values are not broadcasted against non-scalar values, despite what the docstring claims.
fec5dc7
to
b0e0054
Compare
I got rid of the |
x
,y
,fmt
as positional-only.what the docstring claims.
PR Summary
PR Checklist