Split attrs

[pp-mo]

🚀 Pull Request

WIP - just testing, for now

Ultimately, will address #3325

[pp-mo]

Rebased + targetting new feature-branch "FEATURE_split_attrs"

[pp-mo]

UPDATE: I have since converted the rough plan outlined in this comment into tickets on a new project to cover all of this work

---

Note : a rough plan for the future development path...

1. initial tests for status-quo
   • now basically done, though more may still be needed / to be added later
2. cube metadata changes
   • the cube state will contain separate 'local' + 'global' attribute dicts
3. cube attributes handler
   • "cube.attributes" to become a wrapper object, providing access ~same as existing (combined local+global), plus specific local + global settings. (Have code for this prototyped already)
4. netcdf loader changes
   • specifically, assign incoming attributes into relevant local+global areas when loading netcdf
   • NOTE: the 'combined access' means that, at this point, saving to netcdf will have almost-same results.
5. netcdf saver changes
   • changes to respect the local/global designations on saving (e.g. from original load provenance), where possible
   • PLUS: a FUTURE flag implementation to switch new/old handling behaviour
6. ?? cube printout ??
   • COULD modify this to have an extra section.
   • i.e. "Attributes:" (local) and separate "Global Attributes:"
   • not yet clear if this is required / a good idea
     • NB existing listing is presented in alphabetical order
     • so this is going to change A LOT of test results
7. adjust "equalise_attributes"
   • at least ensure that operation makes sense with the new split-form attributes
   • needs to handle newly-possible collisions between matching local+global settings
   • possibly provide automatic resolution of some such problems
   • possibly add extra keywords to control different resolution modes
8. UserGuide update

other questions ...

• ? special additional logic in common-metadata / lenient logic, allowing combine+compare of local+global settings ??
• ? POSSIBLY ? more fixes / rationalisation of existing code
  • adjust LimitedAttributeDict to conform to _CF_ATTRS
  • modify LimitedAttributeDict to complain about mis-assignment of recognised global-only or local-only attributes
    • NOTE: this probably can't / shouldn't occur in the CubeAttributesMap
      • "should" instead occur within the individual (specific) sets
  • worry about handling of "missing_value" attribute : suspect there may be an anomaly here
  • rationalise attribute handling : separate logic to separate code, apart from netcdf-save ??

[pp-mo]

I've now created a project for this entire effort, and re-targetted this PR onto a dedicated feature branch.

So this PR is now represents only the very first step of that (#4981), and should be evaluated, fixed and merged as it is.

Please review for sense, and completeness of the testing scope

[pp-mo]

NOTE: rebased onto updated feature-branch + fixed, following the mergeback which contained #4803

[pp-mo]

@lbdreyer I hope recent commits should cover a lot of your outstanding comments. But a couple of things are clearly still outstanding

• whether I should be testing load- and save behaviours independently of a round-trip approach
  • I suspect you may be right, but changes from this code would be quite involved, so perhaps we should consider it as a separate follow-on.
    I'm still thinking on this one.
• how to fix the parallel tests
  • I have no idea what the problem here is, or why it only seems to occur with Python 3.10, which feels to me like a PyTest problem.
    I will un-fix this + see what happens now. I could also try a rebase to bring in newer dependencies, but AFAICT there is nothing much in the latest iris/main lockfiles that seems like it is going to affect this.

[pp-mo]

Updates ...

@lbdreyer ... a couple of things are clearly still outstanding

* whether I should be [testing load- and save behaviours independently of a round-trip approach](https://github.com/SciTools/iris/pull/4960#discussion_r987733199)

I am working on this. I intend to add some specific load- and save-behaviour tests, probably in addition to the round-trips, but still working out what.

---

* how to [fix the parallel tests](https://github.com/SciTools/iris/pull/4960#discussion_r987285790)

Well, you'll see that I came up with a workaround, but it's really rather horrible ! As we're working on a feature-branch here, I'd rather pause on a better solution for this, and just move forwards noting the problem ?

[pp-mo]

Rebased.

I have now added specific, independent tests for load and save behaviour (file->cubes, cubes->file). These tests should persist in future by matching (almost all) uses of the "combined" cube.attributes value.
And I think ~all results will match, up to but not including the ordering of the dictionary keys.

In future, I hope I can use all the same testcases to check the new cube.attributes.global and cube.attributes.local content in each case.

[pp-mo]

@lbdreyer I think I have now addressed everything outstanding -- can you re-review ? I will then go on to get #5040 rebased onto this

[pp-mo]

Note: when #5095 is merged, that splits tests/integration/test_netcdf.py into various tests/integration/netcdf/test_xxx.py. So we will want to rebase or merge that in here. (Mergeback into the feature-branch would be OK I think)

[pp-mo]

I have now merged-forward latest main into the F-B, and rebased this onto that. So the history here is now much cleaner. This will also affect other open PRs, I think at present only #4983

[pp-mo]

Thanks @lbdreyer I hope recent commits (post-rebase) address all your latest points. The previous notes are also not resolved -- could you please check those over and re-raise anything from there that still seems unfinished ? Otherwise, I hope is all done :crossed_fingers:

[pp-mo]

Great, thanks so much for your patience @lbdreyer !