search

rom-py / rompy

Relocatable Ocean Modelling in PYthon (rompy) combines templated cookie-cutter model configuration with various xarray extensions to assist in the setup and evaluation of coastal ocean model
https://rom-py.github.io/rompy/
BSD 3-Clause "New" or "Revised" License
2 stars 9 forks source link

SourceFiles a wrapper for open_mfdataset #34

Closed benjaminleighton closed 1 year ago

benjaminleighton commented 1 year ago

New SourceFiles similar to SourceFile but please reject and let me know if there is a better alternative.

rafa-guedes commented 1 year ago

@benjaminleighton this looks good, we could merge it - but perhaps another option is to use xr.open_mfdataset in the existing SourceFile class instead of xr.open_dataset. There is one limitation with using xr.open_mfdataset that I am aware of which is it does not handle file-like objects, so if for example we want to open a remote file using fsspec it will throw an exception. We could capture this exception though and attempt to use xr.open_dataset in that case. I'd be happy doing it that way but I'm also happy to have an extra SourceFiles class as you have implemented. What do you think?

benjaminleighton commented 1 year ago

I thought a bit about puting the logic into the SourceFile instead. I'm not sure if there is an issue. https://docs.xarray.dev/en/stable/generated/xarray.open_mfdataset.html and https://docs.xarray.dev/en/stable/generated/xarray.open_dataset.html are different in what kwargs they accept as well. I'm not sure if that matters. Easiest for me is this change as it's what I have 😃 but I don't know if that's a good enough reason really!

benjaminleighton commented 1 year ago

As discussed @rafa-guedes I will abandon this pull request as the new SourceFile functionlity you suggested is being implemented.