-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Additional axis kwargs #2294
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
Additional axis kwargs #2294
Conversation
xarray/tests/test_plot.py
Outdated
da.plot(xincrease=xincrease) | ||
assert(plt.gca().xaxis_inverted() == (not xincrease)) | ||
|
||
@pytest.mark.parametrize('da', test_da_list) |
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.
F811 redefinition of unused 'test_xincrease_kwarg' from line 1686
elif not xincrease: | ||
ax.set_xlim(sorted(ax.get_xlim(), reverse=True)) | ||
elif xincrease and ax.xaxis_inverted(): | ||
ax.invert_xaxis() |
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 there a reason why we weren't using these methods earlier?
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 don't think so!
if np.issubdtype(xval.dtype, np.datetime64): | ||
ax.get_figure().autofmt_xdate() | ||
for xlabels in ax.get_xticklabels(): |
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.
Missed this bit in an older PR.
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.
nice, thanks! LGTM pending the tests are passing
elif not xincrease: | ||
ax.set_xlim(sorted(ax.get_xlim(), reverse=True)) | ||
elif xincrease and ax.xaxis_inverted(): | ||
ax.invert_xaxis() |
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 don't think so!
xarray/plot/plot.py
Outdated
def _update_axes_limits(ax, xincrease, yincrease): | ||
def _update_axes(ax, xincrease, yincrease, | ||
xscale=None, yscale=None, | ||
xticks=None, yticks=None, xlim=None, ylim=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.
aesthetic nit: linebreak after yticks=None,
?
xarray/tests/test_plot.py
Outdated
def test_xincrease_kwarg(self, da, xincrease): | ||
plt.clf() | ||
da.plot(xincrease=xincrease) | ||
assert(plt.gca().xaxis_inverted() == (not xincrease)) |
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 don't even know if it's worth fixing here, but assert
is a statement, not a function. So you can write assert something
rather than assert(something)
.
Don't automatically set histogram ylabel to be 'count'. hist can be used to plot PDFs, in which case that label would be wrong.
As always for plotting functionalities, an example in the gallery would be welcome! This can be done in a subsequent PR of course. |
This looks good to me. @dcherian do you want to do the honors and merge? |
Woot. Thanks. |
whats-new.rst
for all changes andapi.rst
for new API (remove if this change should not be visible to users, e.g., if it is an internal clean-up, or if this is part of a larger project that will be documented later)This PR adds support for
xscale, yscale, xticks, yticks, xlim, ylim
kwargs following discussion in #2224 and the Pandas API (https://pandas.pydata.org/pandas-docs/stable/generated/pandas.DataFrame.plot.html#pandas.DataFrame.plot).Haven't added FacetGrid support yet. I'll get to that soon.