parcels
parcels copied to clipboard
Support time varying depth dimensions for xarray datasets
I just pushed a small fix to the PR; it turned out that the depth fields were never connected to the grid because the Field.dataFiles is None
for Fields created from xarray and therefore the following lines evaluate True https://github.com/OceanParcels/parcels/blob/f6e2ed7f7a2e45eed9acbb2603caf3c124a3c831/parcels/fieldset.py#L271-L272
This seems to be fixed now by changing the last bit to
or (f.dataFiles is None and f.creation_log != 'from_xarray_dataset'):
Can you check if this now works as expected?
I think it does indeed, see the gist ! Thanks for your help.
What next, should we try to adresse the large number of tests that are failing? or work on a tutorial?
I also have a somewhat indirectly related question: could you please confirm using xarray does leverage its lazy capability (i.e. does not load full datasets in memory) ?
Great to hear that this fix seems to work, @apatlpo! I've just pushed a set of fixes (one related to PEP8, the other to a bug in reading the depth
dimension from the dictionary when depth is set) so hopefully the CI should now give green ticks
I don't think we need a tutorial for this, but I've just added a sentence that this works in FieldSet.from_xarray_dataset()
in the timevarying depth dimension tutorial. We do need a unit test though, I'll create that based on your gist and put it in tests/test_grid.py
I also have a somewhat indirectly related question: could you please confirm using xarray does leverage its lazy capability (i.e. does not load full datasets in memory) ?
Hmm, good question. To be honest I don't know. We haven't used FieldSet.from_xarray_dataset()
in real-life applications yet, so had no need to test/check this. I'd be very keen to hear the answer though; hopefully the lazy capabilities work out of the box!
Ok, I'll give a shot to test the lazyness of the code and report back
Ok, I'll give a shot to test the lazyness of the code and report back
Closing this PR for now, as there was no action for a long time