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.

Sign up for GitHub

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

Jump to bottom

Extract cubewrite fill value modifications #114

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Extract cubewrite fill value modifications #114

wants to merge 3 commits into from

Conversation

blimlim
Copy link
Collaborator

@blimlim blimlim commented Sep 27, 2024

This pr closes #99.

It extract's the cubewrite fill value modifications into a seperate function, adds a custom_fill_val optional argument, and adds unit tests.

Any suggestions or ideas are welcome!

@blimlim
Copy link
Collaborator Author

blimlim commented Sep 27, 2024

There's a slight amount of messiness with the added custom_fill_val argument, and so any ideas or suggestions would be great! Specifically, the final line of fix_fill_value forces the fill value to have the same type as the cube.data, which is required by the netCDF conventions

To prevent e.g. a float custom_fill_val from silently being rounded to an integer, I thought it would be good to raise an error when the types don't match:

um2nc-standalone/umpost/um2netcdf.py

Lines 334 to 340 in f3fbd44

if custom_fill_val is not None:
if type(custom_fill_val) == cube.data.dtype:
fill_value = custom_fill_val
else:
msg = (f"custom_fill_val type {type(custom_fill_val)} does not "
f"match cube {cube.name()} data type {cube.data.dtype}.")
raise TypeError(msg)

This will also complain e.g. if a float was going to be rounded down to a float32, however this is stricter than what's applied to the default fill value for floats, which is would get rounded down to a float32 if the the cube's data is float32:

um2nc-standalone/umpost/um2netcdf.py

Lines 342 to 350 in f3fbd44

elif cube.data.dtype.kind == 'f':
fill_value = DEFAULT_FILL_VAL_FLOAT
else:
fill_value = default_fillvals[
f"{cube.data.dtype.kind:s}{cube.data.dtype.itemsize:1d}"
]
# Use an array to force the type to match the data type
cube.attributes['missing_value'] = np.array([fill_value], cube.data.dtype)

I don't think it's too important, but mainly wanted to check whether we're happy for the type requirements for the custom_fill_val to be stricter than what's applied to the default values?

@blimlim blimlim marked this pull request as draft September 27, 2024 01:03
@blimlim
Copy link
Collaborator Author

blimlim commented Sep 27, 2024

Ah oops, didn't notice the additional places the fill_value is required. Will fix this now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@truth-quark truth-quark Awaiting requested review from truth-quark

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extract cubewrite() fill value to function
1 participant
@blimlim