Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DOC: Update and consolidate Custom Tick Formatter for Time Series example #21873
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
DOC: Update and consolidate Custom Tick Formatter for Time Series example #21873
Changes from all commits
31d0f30
5ad9546
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
Maybe just be explicit rather than make the user go back and parse the comment?
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 made this change on purpose to underline/explain that this is an index plot which by definition plots the values versus an index number. In my view, the plot command is more legible/clearer without the range definition as the first argument. Besides, this kind of plot is explicitly advertised in
plot
, so I'd rather keep it as is.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, however, that most of the core devs consider this a nasty holdover from the Matlab days, and prefer the more explicit interface. Certainly when I was reading this I was confused about what
r.adj_close
was, and how it was plotting x and y, and then I had to go up, figure out whatr.adj_close
was and then figure out why it didn't need two arguments.In general, I wonder if it is more confusing than not to keep
r
as a structured array? How common do we think those are in the wild (I never use them, but then I use xarray all the time).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.
hm, I find this feature quite handy, but if this usage is not recommended it should be described as discouraged in the docs, shouldn't it
Alternatives (pandas, xarray, ...) require additional dependencies, csv requires additional date conversion, so why not leave it as a structured array (given that the data structure is described in detail in the comment at lines 24-27)?
This file was deleted.