Skip to content

Conversation

@trexfeathers
Copy link
Contributor

@trexfeathers trexfeathers commented Aug 8, 2023

🚀 Pull Request

Resolves #4989

Description

This is very bare, but as @pp-mo has pointed out there just isn't currently an appropriate place to discuss NetCDF I/O. Raising the profile any further would disproportionately focus on attribute handling and read very strangely. I've made a # TODO: as a placeholder and should probably create an issue tomorrow.


Consult Iris pull request check list

@trexfeathers trexfeathers added the Type: Feature Branch Highlight this for a feature branch label Aug 8, 2023
@trexfeathers trexfeathers requested a review from pp-mo August 8, 2023 16:00
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (68eaa53) 89.41% compared to head (9459c7d) 89.41%.

Additional details and impacted files
@@                 Coverage Diff                  @@
##           FEATURE_split_attrs    #5418   +/-   ##
====================================================
  Coverage                89.41%   89.41%           
====================================================
  Files                       89       89           
  Lines                    22500    22500           
  Branches                  5396     5396           
====================================================
  Hits                     20119    20119           
  Misses                    1636     1636           
  Partials                   745      745           
Files Changed Coverage Δ
lib/iris/cube.py 90.96% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pp-mo
Copy link
Member

pp-mo commented Aug 9, 2023

Closes #4989

@trexfeathers trexfeathers marked this pull request as draft August 9, 2023 10:55
@trexfeathers trexfeathers self-assigned this Aug 16, 2023
@trexfeathers
Copy link
Contributor Author

For the record: I was also planning to create a new technical paper / further topic for NetCDF I/O, add a section on attributes, and make it clear that there was more left to write on other topics. No time to do this in the immediate future.

@trexfeathers trexfeathers marked this pull request as ready for review August 18, 2023 13:51
Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this is sufficient, and that we lack a suitable context to contain a deeper discussion, and that this is not the time to write a general account of netcdf file operations !

I'm happy to merge this as-is now, but we should do a final review of how it all looks before merging back to main.

@pp-mo pp-mo merged commit 57eec4d into SciTools:FEATURE_split_attrs Aug 24, 2023
@pp-mo pp-mo mentioned this pull request Aug 24, 2023
2 tasks
ESadek-MO pushed a commit that referenced this pull request Nov 21, 2023
* Split-attrs: Cube metadata refactortests (#4993)

* Convert Test___eq__ to pytest.

* Convert Test_combine to pytest.

* Convert Test_difference to pytest.

* Review changes.

* Split attrs - tests for status quo (#4960)

* Tests for attribute handling in netcdf load/save.

* Tidy test functions.

* Fix import order exception.

* Add cf-global attributes test.

* Towards more pytest-y implemenation.

* Replace 'create_testcase' with fixture which also handles temporary directory.

* Much tidy; use fixtures to parametrise over multiple attributes.

* Fix warnings; begin data-style attrs tests.

* Tests for data-style attributes.

* Simplify setup fixture + improve docstring.

* No parallel test runner, to avoid error for Python>3.8.

* Fixed for new-style netcdf module.

* Small review changes.

* Rename attributes set 'data-style' as 'local-style'.

* Simplify use of fixtures; clarify docstrings/comments and improve argument names.

* Clarify testing sections for different attribute 'styles'.

* Re-enable parallel testing.

* Sorted params to avoid parallel testing bug - pytest#432.

* Rename test functions to make alpha-order match order in class.

* Split netcdf load/save attribute testing into separate sourcefile.

* Add tests for loaded cube attributes; refactor to share code between Load and Roundtrip tests.

* Add tests for attribute saving.

* Fix method names in comments.

* Clarify source of Conventions attributes.

* Explain the test numbering in TestRoundtrip/TestLoad.

* Remove obsolete test helper method.

* Fix small typo; Fix numbering of testcases in TestSave.

* Implement split cube attributes. (#5040)

* Implement split cube attributes.

* Test fixes.

* Modify examples for simpler metadata printouts.

* Added tests, small behaviour fixes.

* Simplify copy.

* Fix doctests.

* Skip doctests with non-replicable outputs (from use of sets).

* Tidy test comments, and add extra test.

* Tiny typo.

* Remove redundant redefinition of Cube.attributes.

* Add CubeAttrsDict in module __all__ + improve docs coverage.

* Review changes - small test changes.

* More review changes.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix CubeAttrsDict example docstrings.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Odd small fixes.

* Improved docstrings and comments; fix doctests.

* Don't sidestep netcdf4 thread-safety.

* Publicise LimitedAttributeDict, so CubeAttrsDict can refer to it.

* Fix various internal + external links.

* Update lib/iris/cube.py

Co-authored-by: Martin Yeo <[email protected]>

* Update lib/iris/cube.py

Co-authored-by: Martin Yeo <[email protected]>

* Update lib/iris/cube.py

Co-authored-by: Martin Yeo <[email protected]>

* Update lib/iris/cube.py

Co-authored-by: Martin Yeo <[email protected]>

* Streamline docs.

* Review changes.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Martin Yeo <[email protected]>

* Splitattrs ncload (#5384)

* Distinguish local+global attributes in netcdf loads.

* Small test fixes.

* Small doctest fix.

* Fix attribute load-save tests for new behaviour, and old-behaviour equivalence.

* Split attrs docs (#5418)

* Clarification in CubeAttrsDict examples.

* CubeAttrsDict fix docstring typo.

* Raise awareness of split attributes in user guide.

* What's New entry.

* Changes to metadata documentation.

* Splitattrs ncsave redo (#5410)

* Add docs and future switch, no function yet.

* Typing enables code completion for Cube.attributes.

* Make roundtrip checking more precise + improve some tests accordingly (cf. #5403).

* Rework all tests to use common setup + results-checking code.

* Saver supports split-attributes saving (no tests yet).

* Tiny docs fix.

* Explain test routines better.

* Fix init of FUTURE object.

* Remove spurious re-test of FUTURE.save_split_attrs.

* Don't create Cube attrs of 'None' (n.b. but no effect as currently used).

* Remove/repair refs to obsolete routines.

* Check all warnings from save operations.

* Remove TestSave test numbers.

* More save cases: no match with missing, and different cube attribute types.

* Run save/roundtrip tests both with+without split saves.

* Fix.

* Review changes.

* Fix changed warning messages.

* Move warnings checking from 'run' to 'check' phase.

* Simplify and improve warnings checking code.

* Fix wrong testcase.

* Minor review changes.

* Fix reverted code.

* Use sets to simplify demoted-attributes code.

* WIP

* Working with iris 3.6.1, no errors TestSave or TestRoundtrip.

* Interim save (incomplete?).

* Different results form for split tests; working for roundtrip.

* Check that all param lists are sorted.

* Check matrix result-files compatibility; add test_save_matrix.

* test_load_matrix added; two types of load result.

* Finalise special-case attributes.

* Small docs tweaks.

* Add some more testcases,

* Ensure valid sort-order for globals of possibly different types.

* Initialise matrix results with legacy values from v3.6.1 -- all matching.

* Add full current matrix results, i.e. snapshot current behaviours.

* Review changes : rename some matrix testcases, for clarity.

* Splitattrs ncsave redo commonmeta (#5538)

* Define common-metadata operartions on split attribute dictionaries.

* Tests for split-attributes handling in CubeMetadata operations.

* Small tidy and clarify.

* Common metadata ops support mixed split/unsplit attribute dicts.

* Clarify with better naming, comments, docstrings.

* Remove split-attrs handling to own sourcefile, and implement as a decorator.

* Remove redundant tests duplicated by matrix testcases.

* Newstyle split-attrs matrix testing, with fewer testcases.

* Small improvements to comments + docstrings.

* Fix logic for equals expectation; expand primary/secondary independence test.

* Clarify result testing in metadata operations decorator.

* Splitattrs equalise (#5586)

* Add tests in advance for split-attributes handling cases.

* Move dict conversion inside utility, for use elsewhere.

* Add support for split-attributes to equalise_attributes.

* Update lib/iris/util.py

Co-authored-by: Martin Yeo <[email protected]>

* Update lib/iris/tests/unit/util/test_equalise_attributes.py

Co-authored-by: Martin Yeo <[email protected]>

* Simplify and clarify equalise_attributes code.

---------

Co-authored-by: Martin Yeo <[email protected]>

* Fix merge-fail messaging for attribute mismatches. (#5590)

* Extra CubeAttrsDict methods to emulate dictionary behaviours. (#5592)

* Extra CubeAttrsDict methods to emulate dictionary behaviours.

* Don't use staticmethod on fixture.

* Add Iris warning categories to saver warnings.

* Type equality fixes for new flake8.

* Licence header fixes.

* Splitattrs ncsave deprecation (#5595)

* Small improvement to split-attrs whatsnew.

* Emit deprecation warning when saving without split-attrs enabled.

* Stop legacy-split-attribute warnings from upsetting delayed-saving tests.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Martin Yeo <[email protected]>
@trexfeathers trexfeathers deleted the split_attrs_docs branch May 3, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Feature Branch Highlight this for a feature branch

Projects

No open projects
Status: 🏁 Done

Development

Successfully merging this pull request may close these issues.

2 participants