xarray-contrib / cf-xarray

an accessor for xarray objects that interprets CF attributes
https://cf-xarray.readthedocs.io/
Apache License 2.0
157 stars 39 forks source link

Use pooch instead of bundling standard name table #436

Closed dcherian closed 1 year ago

dcherian commented 1 year ago

Closes #422

cc @Zeitsperre

codecov[bot] commented 1 year ago

Codecov Report

Merging #436 (89f1321) into main (2d579f2) will increase coverage by 58.97%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main     #436       +/-   ##
===========================================
+ Coverage   37.35%   96.33%   +58.97%     
===========================================
  Files          21       21               
  Lines        3916     2839     -1077     
  Branches      194        0      -194     
===========================================
+ Hits         1463     2735     +1272     
+ Misses       2259      104     -2155     
+ Partials      194        0      -194     
Flag Coverage Δ
mypy ?
unittests 96.33% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cf_xarray/accessor.py 94.81% <ø> (+58.84%) :arrow_up:
cf_xarray/tests/__init__.py 65.95% <100.00%> (+46.80%) :arrow_up:
cf_xarray/tests/test_accessor.py 99.80% <100.00%> (+70.62%) :arrow_up:
cf_xarray/utils.py 88.57% <100.00%> (+58.57%) :arrow_up:

... and 18 files with indirect coverage changes

malmans2 commented 1 year ago

Another quick comment, if we use fsspec rather than pooch, I think it's more likely that it is already installed as it should be a dependency of dask. See: https://github.com/xarray-contrib/cf-xarray/issues/422#issuecomment-1423034942

Zeitsperre commented 1 year ago

@dcherian Thanks for the heads up!

dcherian commented 1 year ago

if we use fsspec rather than pooch, I think it's more likely that it is already installed as it should be a dependency of dask.

I think its OK. Xarray uses pooch for its tutorial datasets anyway...