Support netCDF load+save on dataset-like objects as well as filepaths. #5214

pp-mo · 2023-03-27T09:30:08Z

Rebased version of #5024

some such feature will be important to the proposed solutions for #4994
But we should remain open to changing how this is implemented.

NOTE: now targets main branch, instead of previous feature-branch FEATURE_xarray_readwrite.
This is because we are no longer planning to host the majority of the 'bridge' code in the Iris repo, but in a separate 'ncdata' repo : see #4994

~~Todo:~~

as in this now- outofdate comment -- ~~make sure all works with paths as well as filepath-strings~~

lib/iris/fileformats/cf.py

codecov · 2023-03-27T09:51:06Z

Codecov Report

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

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

Additional details and impacted files

@@            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

Impacted Files	Coverage Δ
lib/iris/fileformats/netcdf/saver.py	`89.02% <67.85%> (+0.02%)`	⬆️
lib/iris/fileformats/netcdf/_thread_safe_nc.py	`83.55% <70.00%> (+1.03%)`	⬆️
lib/iris/__init__.py	`88.88% <83.33%> (+0.36%)`	⬆️
lib/iris/fileformats/__init__.py	`84.37% <100.00%> (-0.48%)`	⬇️
lib/iris/fileformats/cf.py	`85.65% <100.00%> (+0.11%)`	⬆️
lib/iris/fileformats/netcdf/loader.py	`80.44% <100.00%> (+0.07%)`	⬆️
lib/iris/io/__init__.py	`83.21% <100.00%> (+2.05%)`	⬆️
lib/iris/io/format_picker.py	`83.72% <100.00%> (+2.26%)`	⬆️

... and 1 file with indirect coverage changes

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

pp-mo · 2023-05-10T16:00:21Z

Update

Rebased onto latest main
still WIP

lib/iris/fileformats/netcdf/loader.py

pp-mo · 2023-05-15T14:53:34Z

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.

pp-mo · 2023-05-15T15:13:34Z

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

pp-mo · 2023-05-15T15:16:07Z

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.

ESadek-MO

Hey @pp-mo, looks good, just a couple soft suggestions!

lib/iris/io/__init__.py

lib/iris/fileformats/__init__.py

lib/iris/io/__init__.py

lib/iris/tests/integration/netcdf/test_general.py

pp-mo · 2023-05-17T15:47:05Z

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

…rs use duck typing.

trexfeathers

My thoughts on the changes to _thread_safe_nc.py:

lib/iris/fileformats/netcdf/_thread_safe_nc.py

pp-mo · 2023-05-19T13:17:34Z

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

ESadek-MO

Hey @pp-mo, all looks good now!
Thanks for the review assist @trexfeathers!

pp-mo · 2023-05-19T13:51:00Z

Thanks so much, @ESadek-MO @trexfeathers !

* upstream/main: Updated environment lockfiles (SciTools#5332) Support netcdf variable emulation (SciTools#5212) Support netCDF load+save on dataset-like objects as well as filepaths. (SciTools#5214) minor refinement to release do-nothing script (SciTools#5326) update bibtex citation for v3.6.0 release (SciTools#5324) Whats new updates for v3.6.0 (SciTools#5323) Added whatsnew notes on netcdf delayed saving and Distributed support. (SciTools#5322) Updated environment lockfiles (SciTools#5320) Bugfix 4566 (SciTools#4569) Simpler/faster data aggregation code in `aggregated_by` (SciTools#4970)

* upstream/main: Updated environment lockfiles (SciTools#5334) Iris Docs: Dark theme ready (SciTools#5299) Updated environment lockfiles (SciTools#5332) Support netcdf variable emulation (SciTools#5212) Support netCDF load+save on dataset-like objects as well as filepaths. (SciTools#5214) minor refinement to release do-nothing script (SciTools#5326) update bibtex citation for v3.6.0 release (SciTools#5324) Whats new updates for v3.6.0 (SciTools#5323) Added whatsnew notes on netcdf delayed saving and Distributed support. (SciTools#5322) Updated environment lockfiles (SciTools#5320) Bugfix 4566 (SciTools#4569) Simpler/faster data aggregation code in `aggregated_by` (SciTools#4970) update release_do_nothing script (SciTools#5311) Restore latest Whats New files. Updated environment lockfiles (SciTools#5308) release_do_nothing script updates from v3.6.0rc0 (SciTools#5306) Whats new updates for v3.6.0rc0 (SciTools#5300) Bump scitools/workflows from 2023.04.3 to 2023.05.0 (SciTools#5303)

pp-mo force-pushed the loadsave_ncfiles_2 branch from f503e9d to 6459591 Compare March 27, 2023 09:41

pp-mo commented Mar 27, 2023

View reviewed changes

lib/iris/fileformats/cf.py Show resolved Hide resolved

This was referenced Mar 27, 2023

Load+save things other than filepaths, and specifically support netCDF4.Dataset-s #5024

Closed

Support netcdf variable emulation #5212

Merged

Bridge support #5216

Closed

pp-mo added the Xarray Bridge label Apr 17, 2023

pp-mo self-assigned this May 10, 2023

pp-mo marked this pull request as draft May 10, 2023 15:54

pp-mo force-pushed the loadsave_ncfiles_2 branch from 6459591 to b7ac505 Compare May 10, 2023 15:59

Support netCDF load+save on dataset-like objects as well as filepaths.

81dfd5b

pp-mo force-pushed the loadsave_ncfiles_2 branch from b7ac505 to 81dfd5b Compare May 10, 2023 16:24

pp-mo commented May 10, 2023

View reviewed changes

lib/iris/fileformats/netcdf/loader.py Outdated Show resolved Hide resolved

pp-mo commented May 10, 2023

View reviewed changes

lib/iris/fileformats/netcdf/loader.py Outdated Show resolved Hide resolved

pp-mo commented May 10, 2023

View reviewed changes

lib/iris/fileformats/netcdf/loader.py Show resolved Hide resolved

Forbid completed saves to an open dataset.

4bfd4ab

bjlittle assigned ESadek-MO May 15, 2023

pp-mo added 3 commits May 15, 2023 15:07

Forbid completed saves to an open dataset.

e3aae4b

Check Path usage in netcdf load+save, distinguished from Dataset args.

167dba2

Final adjustments to docs + whatsnew.

0d0116f

pp-mo added 2 commits May 15, 2023 15:55

Fix typos.

22c5350

Ensure defined cube order for stable CDL.

0a7107a

pp-mo marked this pull request as ready for review May 15, 2023 15:12

ESadek-MO reviewed May 17, 2023

View reviewed changes

Review changes.

ecaa700

Save wraps passed datasets in thread-safe wrapper; thread-safe wrappe…

afe5583

…rs use duck typing.

trexfeathers reviewed May 19, 2023

View reviewed changes

lib/iris/fileformats/netcdf/_thread_safe_nc.py Outdated Show resolved Hide resolved

lib/iris/fileformats/netcdf/_thread_safe_nc.py Outdated Show resolved Hide resolved

trexfeathers reviewed May 19, 2023

View reviewed changes

lib/iris/fileformats/netcdf/_thread_safe_nc.py Outdated Show resolved Hide resolved

Review changes.

ce94993

ESadek-MO approved these changes May 19, 2023

View reviewed changes

ESadek-MO merged commit 04dadb2 into SciTools:main May 19, 2023

pp-mo mentioned this pull request May 23, 2023

Working with support in Iris main. pp-mo/ncdata#7

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support netCDF load+save on dataset-like objects as well as filepaths. #5214

Support netCDF load+save on dataset-like objects as well as filepaths. #5214

pp-mo commented Mar 27, 2023 •

edited

Loading

codecov bot commented Mar 27, 2023 •

edited

Loading

pp-mo commented May 10, 2023

pp-mo commented May 15, 2023 •

edited

Loading

pp-mo commented May 15, 2023

pp-mo commented May 15, 2023

ESadek-MO left a comment

pp-mo commented May 17, 2023 •

edited

Loading

trexfeathers left a comment

pp-mo commented May 19, 2023

ESadek-MO left a comment

pp-mo commented May 19, 2023

Support netCDF load+save on dataset-like objects as well as filepaths. #5214

Support netCDF load+save on dataset-like objects as well as filepaths. #5214

Conversation

pp-mo commented Mar 27, 2023 • edited Loading

codecov bot commented Mar 27, 2023 • edited Loading

Codecov Report

pp-mo commented May 10, 2023

Update

pp-mo commented May 15, 2023 • edited Loading

Update

pp-mo commented May 15, 2023

pp-mo commented May 15, 2023

ESadek-MO left a comment

Choose a reason for hiding this comment

pp-mo commented May 17, 2023 • edited Loading

trexfeathers left a comment

Choose a reason for hiding this comment

pp-mo commented May 19, 2023

ESadek-MO left a comment

Choose a reason for hiding this comment

pp-mo commented May 19, 2023

pp-mo commented Mar 27, 2023 •

edited

Loading

codecov bot commented Mar 27, 2023 •

edited

Loading

pp-mo commented May 15, 2023 •

edited

Loading

pp-mo commented May 17, 2023 •

edited

Loading