-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Allow passing axis kwargs to plot #4020
Conversation
Line 125 in 3820fb7
through to Line 384 in 3820fb7
in all the right places (+ tests). This would also solve #3169. |
Hello @raphaeldussin! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-07-01 15:16:02 UTC |
…/xarray into fix_facecolor_plot
apologies for the many commits, testing the test was a bit difficult. |
@mathause is there anything I can do to make this PR move forward? thanks! |
all tests passed! |
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.
thanks for being patient, @raphaeldussin. I'm not too familiar with the plotting code, but I have a few general suggestions.
@keewis thank you for the review. I have implemented the proposed changes. |
thanks, looks good to me. |
would it make sense to update the documentation here: http://xarray.pydata.org/en/stable/plotting.html#maps the update would be: - ax = plt.axes(projection=ccrs.Orthographic(-80, 35))
- air.isel(time=0).plot.contourf(ax=ax, transform=ccrs.PlateCarree())
- ax.set_global()
+ p = air.isel(time=0).plot(projection=ccrs.Orthographic(-80, 35),
+ transform=ccrs.PlateCarree())
+ p.axes.set_global()
@savefig plotting_maps_cartopy.png width=100%
- ax.coastlines()
+ p.axes.coastlines() |
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.
Thanks for the PR. I made suggestions - the main issue I have is that you currently mix kwargs
which should go to ax.plot()
and subplot_kws
which should go to plt.axes()
.
So I think your example should be:
da.plot(subplot_kws=dict(projection=ccrs.PlateCarree(), facecolor="gray"), transform=ccrs.PlateCarree(), ...)
(btw please never use this colormap in production ;-) )
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.
Some more nits from my side.
doc/plotting.rst
Outdated
p = air.isel(time=0).plot(subplot_kws=dict(projection=ccrs.Orthographic(-80, 35), | ||
facecolor="gray"), | ||
transform=ccrs.PlateCarree()) |
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.
p = air.isel(time=0).plot(subplot_kws=dict(projection=ccrs.Orthographic(-80, 35), | |
facecolor="gray"), | |
transform=ccrs.PlateCarree()) | |
p = air.isel(time=0).plot( | |
subplot_kws=dict(projection=ccrs.Orthographic(-80, 35), facecolor="gray"), | |
transform=ccrs.PlateCarree(), | |
) |
nit: formatting
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.
in the future, these kinds of formatting issues in documentation can be automatically fixed using blackdoc
, see #4177.
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.
@mathause: yep, that seems to be restricted to maintainers and pull request authors, so if someone reviewed without being a maintainer they won't be able to change the resolve status of their own comments. Also, there doesn't seem to be a setting to change this. |
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.
Thanks @raphaeldussin. I see this is your first PR , welcome!
I apologize for the late review. Thank you for your patience.
I have some minor comments: mostly to clean up the tests you've added (for a previously untested function, so thanks!)
this should be good to go when tests pass. |
* upstream/master: (21 commits) fix typo in error message in plot.py (pydata#4188) Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods (pydata#3936) Show data by default in HTML repr for DataArray (pydata#4182) Blackdoc (pydata#4177) Add CONTRIBUTING.md for the benefit of GitHub Correct dask handling for 1D idxmax/min on ND data (pydata#4135) use assert_allclose in the aggregation-with-units tests (pydata#4174) Remove old auto combine (pydata#3926) Fix 4009 (pydata#4173) Limit length of dataarray reprs (pydata#3905) Remove <pre> from nested HTML repr (pydata#4171) Proposal for better error message about in-place operation (pydata#3976) use builtin python types instead of the numpy alias (pydata#4170) Revise pull request template (pydata#4039) pint support for Dataset (pydata#3975) drop eccodes in docs (pydata#4162) Update issue templates inspired/based on dask (pydata#4154) Fix failing upstream-dev build & remove docs build (pydata#4160) Improve typehints of xr.Dataset.__getitem__ (pydata#4144) provide a error summary for assert_allclose (pydata#3847) ...
@dcherian thanks for the final fixes! |
@raphaeldussin thanks for your patience here. |
* master: Add initial cupy tests (pydata#4214) Add 0.16.0 release summary New whatsnew section Release v0.16.0 Minor reorg of whatsnew for 0.16.0 (pydata#4216) fix sphinx warnings (pydata#4199) pin isort (pydata#4206) get the colorbar label via public methods (pydata#4201) Bump minimum versions for 0.16 release (pydata#4175) Allow passing axis kwargs to plot (pydata#4020) Fix to_unstacked_dataset for single dimension variables. (pydata#4094) Improve the speed of from_dataframe with a MultiIndex (by 40x!) (pydata#4184) More pint compatibility: silence UnitStrippedWarnings (pydata#4163) Fix typo (pydata#4192) use the latest image of RTD (pydata#4191)
facecolor can be passed in plot kwargs but has no effect and does not raise exception.
PR fixes the problem.