Skip to content

MAINT: improver save refactor and support for overriding all arguments - #2412

Open
cpelley wants to merge 9 commits into
masterfrom
chunk_param
Open

MAINT: improver save refactor and support for overriding all arguments#2412
cpelley wants to merge 9 commits into
masterfrom
chunk_param

Conversation

@cpelley

@cpelley cpelley commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

This change retains support for existing use of save while aligning it with iris.fileformats.netcdf.save
Enables projects which utilise this function to override the chunking in particular (motivated by EPP).

  • save_netcdf now passes kwargs to the underlying iris.fileformats.netcdf.save function.
  • Total refactor of the function as there were significant pre-existing issues:
    • Derivation of chunksizes is now constrained properly without making assumptions of which dimensions the grid are mapped to, or assuming consistent grid shape between cubes within the CubeList.
    • Constrained to dimension grid coordinates.
    • save_netcdf call permutations missing from testing are now tested.
    • compression_level keyword argument from save_netcdf still supported and takes precedence but is deprecated in order to better align with its name from iris.fileformats.netcdf.save (i.e. complevel).
    • with_output from improver/cli/__init__.py has been updated to call save_netcdf with complevel but compression_level keyword argument has been kept the same here so as not to break existing workflows that are reliant on the cli (clize, paraflow).

@cpelley cpelley self-assigned this Jul 16, 2026
@cpelley
cpelley force-pushed the chunk_param branch 2 times, most recently from 4ef64eb to 79b0a0c Compare July 16, 2026 12:17
@cpelley cpelley mentioned this pull request Jul 16, 2026
3 tasks
@cpelley
cpelley requested a review from JoshuaWiggs July 16, 2026 13:53
@cpelley
cpelley marked this pull request as ready for review July 16, 2026 13:59
Copilot AI review requested due to automatic review settings July 16, 2026 13:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors improver.utilities.save.save_netcdf to better align with iris.fileformats.netcdf.save, primarily by forwarding arbitrary keyword arguments and documenting where IMPROVER’s defaults differ (eg compression/chunking defaults).

Changes:

  • Reworks save_netcdf to accept **kwargs for forwarding to iris.fileformats.netcdf.save, including chunking overrides.
  • Introduces argument-normalisation for compression (compression_level deprecation in favor of complevel) and for zlib / shuffle defaults.
  • Uses as_cubelist to standardise cube/cubelist input handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread improver/utilities/save.py
Comment thread improver/utilities/save.py Outdated
Comment thread improver/utilities/save.py Outdated
Comment thread improver/utilities/save.py Outdated
Comment thread improver/utilities/save.py Outdated
@cpelley
cpelley marked this pull request as draft July 20, 2026 11:28
@cpelley

cpelley commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Putting this back into draft for the mo. @JoshuaWiggs
The existing chunksize calculation is making wide assumptions.
Making more changes and I don't feel comfortable to leave at it was.

@cpelley

cpelley commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Nearly there. Interestingly just dealing with an erroneous segmentation fault in the unittests...

=== 7829 passed, 793 skipped, 1 xpassed, 3481 warnings in 208.39s (0:03:28) ====
Fatal Python error: Segmentation fault
Current thread 0x00007f7fdf075440 (most recent call first):
  Garbage-collecting
  <no Python frame>
/home/runner/work/_temp/69157979-fe09-48e0-af0b-b6ae4177db62: line 2:  2575 Segmentation fault      (core dumped) python -u -X faulthandler -m pytest -p no:faulthandler

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

improver/utilities/save.py:109

  • save_netcdf's new parameters (zlib, shuffle, chunksizes) are positional by default, which can silently mis-route existing positional calls from the old signature (eg a previous least_significant_digit positional arg would now be interpreted as zlib). Consider making all parameters after complevel keyword-only to avoid accidental positional misuse.
    complevel: int = 1,
    zlib: bool | None = None,
    shuffle: bool = True,
    chunksizes: tuple | None = None,
    **kwargs,

Comment thread .github/workflows/ci.yml
Comment thread improver/utilities/save.py Outdated
Comment thread improver/utilities/save.py
Comment thread improver_tests/utilities/test_save.py
Comment thread improver_tests/utilities/test_save.py
Comment thread improver_tests/utilities/test_save.py Outdated
Comment thread improver_tests/utilities/test_save.py
Comment thread improver_tests/utilities/test_save.py
Comment thread improver_tests/utilities/test_save.py
@cpelley
cpelley requested a review from Anzerkhan27 July 20, 2026 23:58
@cpelley
cpelley marked this pull request as ready for review July 21, 2026 00:00
@cpelley cpelley changed the title MAINT: Refactored improver save to support overriding all arguments MAINT: improver save refactor and support for overriding all arguments Jul 21, 2026
@cpelley
cpelley requested a review from mo-robert-purvis July 27, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants