-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
DOC: Fixed x, y, docstring in errorbar #8139
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2705,11 +2705,11 @@ def errorbar(self, x, y, yerr=None, xerr=None, | |
|
|
||
| Parameters | ||
| ---------- | ||
| x : scalar | ||
| y : scalar | ||
| x : scalar or array-like | ||
| y : scalar or array-like | ||
|
|
||
| xerr/yerr : scalar or array-like, shape(n,1) or shape(2,n), optional | ||
| If a scalar number, len(N) array-like object, or an Nx1 | ||
| xerr/yerr : scalar or array-like, shape(N,) or shape(2,N), optional | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change gives me pause as there is a bunch of subtle logic in error bar that depends on this shape. Can the exact behavior of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I did some checks and the docs are 99% correct. I am submitting an issue right now about an inconsistency that I found.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is the issue: #8140
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The shape is currently checked with while the following does not: However, this does work: So technically,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went ahead and added this pair of examples to issue #8140 |
||
| If a scalar number, len(N) array-like object, or a N-element | ||
| array-like object, errorbars are drawn at +/-value relative | ||
| to the data. Default is None. | ||
|
|
||
|
|
||
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 would just say "array-like", scalars is just a special case.
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'd keep it this way. Array-like refers to iterable, which a scalar isn't.
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.
scatter,bar,stepall usearray_likein their docs even though they also accept scalars. Either way we should be consistent.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 think that it should be removed for
xandybut retained forerrxanderry.xandyare arrays to plot. As @anntzer says, scalars are just a special case that represents a dataset of size 1. Forerrxanderry, scalars have a somewhat special meaning, which should be emphasized in the docs. Will update.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.
Another argument in @anntzer's favor is that the scalar case is explicitly dealt with in the sentence above the parameters section.
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.
e.g.
np.sum: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.
As I said, I really don't care as I consider Matplotlib's loose input format harmful and not useful for the project, but this convention is not only unclear, but not followed by major scientific projects (scipy, sklearn, skimage, sympy pandas and even Matplotlib).
It boils down whether you want or not to document the fact that errorbar takes scalars as input. I am, once again, totally fine with not documenting it, but let's not pretend that the current docstring documents this in any meaningful and clear way.
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.
The previous sentence reads:
I think that should be enough for most users.
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 think that's good enough.
FWIW git-grepping the codebase of scipy and skimage yields no result for
... or array-likeorarray-like or ...(where ... refers to "scalar", "float", or some similar term); meanwhile at least some functions that are documented to take an "array-like" as argument can also take a scalar.scikit-learn is different because it documents the shape of the required input nearly every time it documents an input as array-like, so scalars are explicitly excluded by that shape info.
sympy uses the word array-like exactly twice in its docs (in the sense of not including scalars).
pandas seems to use the word array-like in the sense of not including scalars as well.
So overall I wouldn't say there's any global agreement.
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.
Since this thread is based on an outdated diff, I went ahead and changed back to
scalar or array-like. I think that is a reasonable compromise that does not sacrifice clarity, which is really the main objective here (more so than consistency in my opinion).