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

Clean, test & refactor 64 to 32 bit conversion step. #96

Merged
merged 11 commits into from
Sep 12, 2024

Conversation

truth-quark
Copy link
Collaborator

Resolves #95.

This PR is mostly to extract & test 64 to 32 bit down-conversion from cubewrite().

I've added a few things:

  • New data member to DummyCube, so data grids can be attached to imitation cubes (it's simpler to reuse fixtures).
  • Unit tests
  • Added integer overflow warning (just in case)

I'll defer moving this step into process() until cubewrite() is deconstructed.

Comments welcome :-)

@truth-quark truth-quark self-assigned this Sep 6, 2024
test/test_um2netcdf.py Outdated Show resolved Hide resolved
test/test_um2netcdf.py Outdated Show resolved Hide resolved
test/test_um2netcdf.py Outdated Show resolved Hide resolved
umpost/um2netcdf.py Outdated Show resolved Hide resolved
umpost/um2netcdf.py Outdated Show resolved Hide resolved
blimlim
blimlim previously approved these changes Sep 10, 2024
Copy link
Collaborator

@blimlim blimlim left a comment

Choose a reason for hiding this comment

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

Everything looks good to me!

@truth-quark
Copy link
Collaborator Author

@marc-white did you want to do a final skim/review of the "end" product?

@marc-white
Copy link

Looks good to me. I just tried to resolve the last outstanding comment, but Github is being difficult... maybe the VSCode extension can save me...

Copy link

@marc-white marc-white left a comment

Choose a reason for hiding this comment

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

Improvements looks good, should be set!

test/test_um2netcdf.py Outdated Show resolved Hide resolved
@truth-quark
Copy link
Collaborator Author

Looks good to me. I just tried to resolve the last outstanding comment, but Github is being difficult... maybe the VSCode extension can save me...

I could close the convo off...

@truth-quark
Copy link
Collaborator Author

truth-quark commented Sep 12, 2024

Awesome, I think closing the open conversation thread off removed review approvals. @marc-white or @blimlim would either of you mind doing another approval to enable a merge?

Copy link
Collaborator

@blimlim blimlim left a comment

Choose a reason for hiding this comment

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

Looks good!

@truth-quark truth-quark merged commit 92096ce into main Sep 12, 2024
4 checks passed
@truth-quark truth-quark deleted the 95/refactor-32bit branch October 1, 2024 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor: cubewrite() extract conversion from 64 to 32 bit values
3 participants