Codecov Report

Patch coverage: 81.81% and project coverage change: +0.02 🎉

Comparison is base (b035094) 89.32% compared to head (ce94993) 89.34%.

@@            Coverage Diff             @@
##             main    #5214      +/-   ##
==========================================
+ Coverage   89.32%   89.34%   +0.02%     
==========================================
  Files          89       89              
  Lines       22392    22413      +21     
  Branches     5375     5380       +5     
==========================================
+ Hits        20002    20026      +24     
+ Misses       1640     1639       -1     
+ Partials      750      748       -2     

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Update

Rebased onto latest main
still WIP

Update

I discovered that this feature now has a problem with output to a already-open dataset (one owned by the caller) whencompute=True (which is the default!) :

• since we added support for distributed schedulers in Lazy netcdf saves #5191, lazy streaming requires the original file to be closed -- because the dask workers doing the streaming may now be separate processes, not always threads, which means they must each (re-)open the dataset for writing -- which they can't if the original caller still has it open
• this simply isn't compatible with saving to an open dataset, because iris.save doesn't have the right to close the dataset (since I don't think netCDF4 supports "re-opening" a dataset, and closing will invalidate any netCDF4 objects held by user).

So that means that, in most cases, you can't just iris.save(cubes, my_dataset).
You must instead saves = iris.save(cubes, my_dataset, compute=False); my_dataset.close(); saves.compute()

Following offline discussion with @bjlittle @trexfeathers

• We can either

  • forbid this usage, or
  • error out except in those specific cases which happen to be OK -- e.g. when dataset is not a real file-based dataset (as intended for 4994 ), or if all data is real.

• we decided that the usecase is so specialist that it will probably only be used for Xarray bridge #4994 support. Hence we should ban compute=True operation saving to a provided dataset, since the feature is still useable (just less convenient in a rare usecase) and it is the simplest solution, both to implement and document.

Ping @ESadek-MO
Where you going to review this (:crossed_fingers: :pray: ) ?
I think it is at last ready to go !

Ping @ESadek-MO Where you going to review this (crossed_fingers pray ) ? I think it is at last ready to go !

P.S. #5212 will be a follow-up, which should complete essential support for #4994.
But that should be a lot simpler than this.

Thanks @ESadek-MO
I think I addressed all those points now -- please re-review + see what you think.

Thanks @trexfeathers
I think I've responded to all of those, now.
Please re-review !

Thanks so much, @ESadek-MO @trexfeathers !