JiaweiZhuang / xESMF

Universal Regridder for Geospatial Data
http://xesmf.readthedocs.io/
MIT License
269 stars 49 forks source link

Added keep_attrs option for xarray regridding #67

Closed raspstephan closed 4 years ago

raspstephan commented 4 years ago

As mentioned in #66 this pull request adds the option to keep the attributes of xarray objects after regridding. Because __call__ checks for the input datatype and selects different regrid functions, some of which are not xarray objects (and therefore keep_attrs would be meaningless), I implemented the arguments using **kwargs.

I realize this is not particularly nice, since the argument doesn't show up, e.g. when using autocomplete. The other option would be to do something like:

if isinstance(indata, np.ndarray):
        return self.regrid_numpy(indata)
elif isinstance(indata, xr.DataArray):
        return regrid_dataarray(indata, keep_attrs=keep_attrs)

Would this be better?

codecov-io commented 4 years ago

Codecov Report

Merging #67 into master will decrease coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
- Coverage   95.76%   95.75%   -0.02%     
==========================================
  Files           6        6              
  Lines         260      259       -1     
==========================================
- Hits          249      248       -1     
  Misses         11       11
Impacted Files Coverage Δ
xesmf/frontend.py 93.93% <100%> (-0.05%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 35c34a6...ea4d68d. Read the comment docs.

JiaweiZhuang commented 4 years ago

I realize this is not particularly nice, since the argument doesn't show up, e.g. when using autocomplete.

Yeah I think the explicit one without **kwargs is better. Could you update it? It is fine for keep_attrs to be meanless for numpy inputs. When using apply_ufunc on numpy arrays, most kwargs including keep_attrs also get ignored.

raspstephan commented 4 years ago

I refactored the code which now uses the explicit version.