Allow passing axis kwargs to plot #4020

raphaeldussin · 2020-05-02T17:36:27Z

facecolor can be passed in plot kwargs but has no effect and does not raise exception.
PR fixes the problem.

mathause · 2020-05-03T08:45:58Z

facecolor is a valid argument for several of the plotting functions (e.g. pcolormesh). The better approach here would be to thread subplot_kws

xarray/xarray/plot/plot.py

Line 125 in 3820fb7

subplot_kws=None,

through to get_axis

xarray/xarray/plot/utils.py

Line 384 in 3820fb7

def get_axis(figsize, size, aspect, ax):

in all the right places (+ tests). This would also solve #3169.

pep8speaks · 2020-05-16T19:06:57Z

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

raphaeldussin · 2020-05-16T23:26:42Z

apologies for the many commits, testing the test was a bit difficult.

raphaeldussin · 2020-05-17T00:33:52Z

an example of using cartopy and a custom background:

raphaeldussin · 2020-06-12T19:36:36Z

@mathause is there anything I can do to make this PR move forward?
I fixed the conflict that happened since my last commit. failed tests here seem independent from PR.

thanks!

raphaeldussin · 2020-06-23T21:00:47Z

all tests passed!

keewis

thanks for being patient, @raphaeldussin. I'm not too familiar with the plotting code, but I have a few general suggestions.

xarray/plot/utils.py

xarray/tests/test_plot.py

xarray/plot/utils.py

xarray/tests/test_plot.py

raphaeldussin · 2020-06-23T21:35:57Z

@keewis thank you for the review. I have implemented the proposed changes.

keewis · 2020-06-23T21:51:55Z

thanks, looks good to me.

raphaeldussin · 2020-06-24T17:12:16Z

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()

mathause

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 ;-) )

xarray/plot/utils.py

xarray/plot/plot.py

xarray/tests/test_plot.py

…/xarray into fix_facecolor_plot

mathause

Some more nits from my side.

mathause · 2020-06-25T08:17:06Z

doc/plotting.rst

+    p = air.isel(time=0).plot(subplot_kws=dict(projection=ccrs.Orthographic(-80, 35),
+                                               facecolor="gray"),
+                              transform=ccrs.PlateCarree())


Suggested change

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

in the future, these kinds of formatting issues in documentation can be automatically fixed using blackdoc, see #4177.

xarray/plot/plot.py

xarray/plot/utils.py

xarray/tests/test_plot.py

xarray/plot/plot.py

mathause

Looks good to me - thanks.

The "resolve conversation" button seems to have disappeared... so I cannot close the comments that are no longer relevant.

pinging @keewis @dcherian

raphaeldussin · 2020-06-25T16:12:27Z

Looks good to me - thanks.

The "resolve conversation" button seems to have disappeared... so I cannot close the comments that are no longer relevant.

pinging @keewis @dcherian

I can see the "resolve conversation" buttons, do you want me to close them?

xarray/plot/plot.py

keewis · 2020-06-25T16:39:39Z

@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.

dcherian

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!)

xarray/tests/test_plot.py

dcherian · 2020-06-29T21:48:34Z

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) ...

raphaeldussin · 2020-07-01T15:30:42Z

@dcherian thanks for the final fixes!

dcherian · 2020-07-02T20:51:51Z

@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)

fix facecolor plot

0d4d29a

raphaeldussin added 4 commits May 15, 2020 18:06

temp version

04a48c4

finish fix facecolor + solves pydata#3169

2577ddd

black formatting

c5c7732

add testing

9d259e2

raphaeldussin added 10 commits May 16, 2020 15:37

allow cartopy projection to be a kwarg

2d7cf27

fix PEP8 comment

19b59a3

black formatting

d439f6b

fix testing, plt not in parameterize

2d5b599

fix testing, allows for no matplotlib

4d00aeb

Merge branch 'fix_facecolor_plot' of https://github.com/raphaeldussin…

7c80566

…/xarray into fix_facecolor_plot

black formating

ba36f83

fix tests without matplotlib

f7c48be

fix some mistakes

180d451

isort, mypy

e8abe18

fix mypy

7c46b01

Merge remote-tracking branch 'xarray/master' into fix_facecolor_plot

2739599

remove empty line

5f30e7c

keewis reviewed Jun 23, 2020

View reviewed changes

correction from review

aebb441

mathause reviewed Jun 24, 2020

View reviewed changes

raphaeldussin added 5 commits June 24, 2020 19:09

correction from 2nd review

38fbfde

updated tests

66e07df

updated tests

3a12e86

Merge branch 'fix_facecolor_plot' of https://github.com/raphaeldussin…

cdeea2f

…/xarray into fix_facecolor_plot

black formatting

ee25c00

mathause reviewed Jun 25, 2020

View reviewed changes

xarray/plot/plot.py Outdated Show resolved Hide resolved

follow up correction from review

7ae74fe

mathause approved these changes Jun 25, 2020

View reviewed changes

dcherian reviewed Jun 25, 2020

View reviewed changes

xarray/plot/plot.py Outdated Show resolved Hide resolved

dcherian reviewed Jun 25, 2020

View reviewed changes

xarray/tests/test_plot.py Outdated Show resolved Hide resolved

xarray/tests/test_plot.py Outdated Show resolved Hide resolved

xarray/tests/test_plot.py Outdated Show resolved Hide resolved

raphaeldussin added 3 commits June 25, 2020 13:15

fix tests

a049209

fix tests again

98264bd

fix bug in tests

79b08db

keewis reviewed Jun 25, 2020

View reviewed changes

xarray/tests/test_plot.py Outdated Show resolved Hide resolved

raphaeldussin and others added 3 commits June 25, 2020 13:39

fix pb in tests

f54d585

remove useless line

1cec6cd

clean up tests

073c64b

dcherian changed the title ~~fix facecolor plot~~ Allow passing axis kwargs to plot Jun 29, 2020

dcherian added 3 commits June 29, 2020 17:26

fix

a652843

Add whats-new

d65416a

dcherian merged commit 834d4c4 into pydata:master Jul 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow passing axis kwargs to plot #4020

Allow passing axis kwargs to plot #4020

raphaeldussin commented May 2, 2020

mathause commented May 3, 2020

pep8speaks commented May 16, 2020 •

edited

raphaeldussin commented May 16, 2020 •

edited

raphaeldussin commented May 17, 2020

raphaeldussin commented Jun 12, 2020

raphaeldussin commented Jun 23, 2020

keewis left a comment

raphaeldussin commented Jun 23, 2020

keewis commented Jun 23, 2020

raphaeldussin commented Jun 24, 2020

mathause left a comment

mathause left a comment

mathause Jun 25, 2020

keewis Jun 25, 2020

mathause left a comment

raphaeldussin commented Jun 25, 2020

keewis commented Jun 25, 2020

dcherian left a comment

dcherian commented Jun 29, 2020

raphaeldussin commented Jul 1, 2020

dcherian commented Jul 2, 2020

Allow passing axis kwargs to plot #4020

Allow passing axis kwargs to plot #4020

Conversation

raphaeldussin commented May 2, 2020

mathause commented May 3, 2020

pep8speaks commented May 16, 2020 • edited

Comment last updated at 2020-07-01 15:16:02 UTC

raphaeldussin commented May 16, 2020 • edited

raphaeldussin commented May 17, 2020

raphaeldussin commented Jun 12, 2020

raphaeldussin commented Jun 23, 2020

keewis left a comment

Choose a reason for hiding this comment

raphaeldussin commented Jun 23, 2020

keewis commented Jun 23, 2020

raphaeldussin commented Jun 24, 2020

mathause left a comment

Choose a reason for hiding this comment

mathause left a comment

Choose a reason for hiding this comment

mathause Jun 25, 2020

Choose a reason for hiding this comment

keewis Jun 25, 2020

Choose a reason for hiding this comment

mathause left a comment

Choose a reason for hiding this comment

raphaeldussin commented Jun 25, 2020

keewis commented Jun 25, 2020

dcherian left a comment

Choose a reason for hiding this comment

dcherian commented Jun 29, 2020

raphaeldussin commented Jul 1, 2020

dcherian commented Jul 2, 2020

pep8speaks commented May 16, 2020 •

edited

raphaeldussin commented May 16, 2020 •

edited