Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

split-attrs: netcdf saver #4986

Closed
pp-mo opened this issue Sep 23, 2022 · 4 comments · Fixed by #5410
Closed

split-attrs: netcdf saver #4986

pp-mo opened this issue Sep 23, 2022 · 4 comments · Fixed by #5410
Assignees
Labels
Type: Feature Branch Highlight this for a feature branch

Comments

@pp-mo
Copy link
Member

pp-mo commented Sep 23, 2022

  • changes to netcdf Saver code
    • when saving to nc files, make it respect the local/global status of attributes where possible
      (i.e. typically, if loaded from an original nc file, the local/global status as it was there -- cf split-attrs: netcdf loader #4984 )
  • (plus) : a new FUTURE flag implementation to switch between new/old handling behaviour
@pp-mo
Copy link
Member Author

pp-mo commented Sep 23, 2022

In this case, we should get some different behaviour -- at last !
And we will want to expand the tests.

First, I'm expecting to add a new Pytest parameter over "FUTURE.nc_save_splitattrs = (False, True)" to existing tests.
I'm hoping that the differences between old+new results can be captured in the existing testcases (cf #4981)

  • duplicate old+new tests with parameterisation
  • make testcases switch on parameter, to capture differences between old+new

Also, some specific new behaviour might require additional tests, to run only with "new-style" saving.

@pp-mo pp-mo changed the title netcdf saver changes split-attrs: netcdf saver Sep 23, 2022
@pp-mo pp-mo added the Type: Feature Branch Highlight this for a feature branch label Sep 26, 2022
@pp-mo
Copy link
Member Author

pp-mo commented Apr 18, 2023

NOTE: as noted here, I think the special status of attributes called "conventions" (small-c) is probably a mistake, and can be remove.

It was introduced in #711 -- before that, there was already code which would handle "conventions" or "Conventions" specially, using "str.lower()" in several places.
I don't believe lower-case-"conventions" itself was ever a CF term, and I suspect this was probably just defensive programming.
Goes all the way back to the initial commit !

@github-project-automation github-project-automation bot moved this to 🆕 New - potential tasks in 🦊 Iris v3.7.0 Jul 4, 2023
@pp-mo pp-mo moved this from 🆕 New - potential tasks to 📋 Backlog in 🦊 Iris v3.7.0 Jul 4, 2023
@pp-mo pp-mo moved this from Todo to In Progress in Split cube-attrs project Jul 22, 2023
@pp-mo pp-mo moved this from 📋 Backlog to 🏗 In progress in 🦊 Iris v3.7.0 Jul 24, 2023
@pp-mo
Copy link
Member Author

pp-mo commented Aug 2, 2023

#5410 is now the PR for this.

@pp-mo pp-mo closed this as completed Aug 4, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to 🏁 Done in 🦊 Iris v3.7.0 Aug 4, 2023
@pp-mo pp-mo reopened this Aug 4, 2023
@github-project-automation github-project-automation bot moved this from 🏁 Done to 📋 Backlog in 🦊 Iris v3.7.0 Aug 4, 2023
@pp-mo pp-mo moved this from 📋 Backlog to 👀 In review in 🦊 Iris v3.7.0 Aug 9, 2023
@pp-mo pp-mo mentioned this issue Aug 9, 2023
3 tasks
@pp-mo pp-mo moved this from 👀 In review to 🔖Assigned in 🦊 Iris v3.7.0 Aug 15, 2023
@pp-mo pp-mo moved this from 🔖Assigned to 🏗 In progress in 🦊 Iris v3.7.0 Aug 15, 2023
@pp-mo pp-mo moved this from 🏗 In progress to 🔖Assigned in 🦊 Iris v3.7.0 Aug 15, 2023
@pp-mo pp-mo moved this from In Progress to Ready / In Review in Split cube-attrs project Aug 22, 2023
@pp-mo pp-mo linked a pull request Aug 22, 2023 that will close this issue
3 tasks
@pp-mo pp-mo moved this from Ready / In Review to Done in Split cube-attrs project Oct 10, 2023
@pp-mo
Copy link
Member Author

pp-mo commented Nov 22, 2023

closed by #5410

@pp-mo pp-mo closed this as completed Nov 22, 2023
@github-project-automation github-project-automation bot moved this from 🔖Assigned to 🏁 Done in 🦊 Iris v3.7.0 Nov 22, 2023
@scitools-ci scitools-ci bot removed this from 🚴 Peloton Dec 15, 2023
@scitools-ci scitools-ci bot removed this from 🚴 Peloton Dec 21, 2023
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 a pull request may close this issue.

2 participants