search

intake / intake-xarray

Intake plugin for xarray
https://intake-xarray.readthedocs.io/
BSD 2-Clause "Simplified" License
74 stars 36 forks source link

added mfdataset to opendap #113

Closed kthyng closed 2 years ago

kthyng commented 2 years ago

Hi! I would like to use mfdataset with opendap and I put it in really simply. I wasn't sure how to incorporate store like in open_dataset, if that is possible. Also, I ran tests locally but on the master branch I had 5 errors, and the same 5 errors with this change so I figured it was something to do with my local configuration rather than the changes I made.

aaronspring commented 2 years ago

I am surprised that works on urls. Could you add a test for urlpath as list and another containing ?

kthyng commented 2 years ago

@aaronspring Ah, you are right about that — "*" will not work with opendap (I copied it uncritically from the open_netcdf function). I removed that.

I added a test for open_opendap with a single url and a list of two urls.

What do you think?

aaronspring commented 2 years ago

Ok. So the passing of kwargs for either mfdataset or dataset is now implicitly set by the type of urlpath. list makes mfdataset and allows parallel=True.

works for me.

Does that solution also work for your intake catalog with 127 datasets?

martindurant commented 2 years ago

I'm only watching this thread, please ping me if you need anything

kthyng commented 2 years ago

@aaronspring Yep! It works with a big list of remote files like as described in the intake-thredds issue except doesn't rely on intake-thredds.

Any idea why some checks were not successful on this PR?

aaronspring commented 2 years ago

upstream issues are not your fault. same problem in https://github.com/intake/intake-xarray/pull/109 mentioned also in https://github.com/rasterio/rasterio/issues/2381

kthyng commented 2 years ago

Ok, just let me know if I should do anything to move this forward! Thanks @aaronspring and @martindurant.

aaronspring commented 2 years ago

I agree with this PR. But cannot merge as I am not a maintainer here. Can you @martindurant? Or should we wait until the rasterio issue is solved?

martindurant commented 2 years ago

I suppose rasterio is unrelated, so I can merge this now. Would you like to be a maintainer here, @aaronspring ?

aaronspring commented 2 years ago

I'd be happy to join @martindurant

kthyng commented 2 years ago

Thank you @aaronspring and @martindurant!

aaronspring commented 2 years ago

Thank you @kthyng for this addition