Skip to content

deps: remove usage of serialbox in pyFV3#121

Merged
oelbert merged 2 commits intoNOAA-GFDL:developfrom
romanc:romanc/serialbox-dependency
Mar 10, 2026
Merged

deps: remove usage of serialbox in pyFV3#121
oelbert merged 2 commits intoNOAA-GFDL:developfrom
romanc:romanc/serialbox-dependency

Conversation

@romanc
Copy link
Collaborator

@romanc romanc commented Mar 3, 2026

Description

Serialbox is a serialization library that is used to build a validation framework against reference runs. At NASA/NOAA, we use this branch, which includes a couple of custom commands.

The currently configured serialbox package is about generating and distributing serial numbers. The fact that this misconfiguration never bothered anyone is surprising and hints at the fact that the serialbox serialization library is probably unused as part of the pyFV3 installation.

This PR suggests to remove that dependency and delete the one example file which has serialbox as a dependency. According to @FlorianDeconinck the example file can't run anymore in this configuration because the data saved with serialbox isn't available anymore. An alternative would be to keep the example file and update it to run with currently available data. Either way, we wouldn't need serialbox as a dependency because now we usually serialize to netcdf.

/cc @FlorianDeconinck as discussed

How Has This Been Tested?

Self-review of code plus passing existing tests.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas: N/A
  • I have made corresponding changes to the documentation: N/A
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included: N/A
  • Targeted model if this changed was triggered by a model need/shortcoming: N/A

@romanc romanc requested review from fmalatino and jjuyeonkim March 3, 2026 08:07
Copy link
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

Pulling back here - we should not have serialbox as a dependency at all.

@oelbert : the only use of the serialbox python package is in acoustics.py, a one-file example on how to use the acoustics time step that uses some long lost serialbox data: can we burn that file?

fmalatino
fmalatino previously approved these changes Mar 5, 2026
romanc and others added 2 commits March 9, 2026 09:10
Serialbox is a serialization library that is used to build a validation
framework against reference runs.

The library is not strictly needed to run tests. I thus removed it from
the tests extra. It is used to serialize data and can thus be helpful to
be installed with pyFV3. I thus left it as an optional dependency.

The currently configured "serialbox" package is about generating
and distributing serial numbers. The fact that this misconfiguration
never bothered anyone is surprising and hints at the fact that the
serialbox serialization library is probably unused as part of the pyFV3
installation.
@romanc romanc force-pushed the romanc/serialbox-dependency branch from 8325902 to b93ac15 Compare March 9, 2026 08:13
@romanc romanc requested a review from oelbert March 9, 2026 08:13
@romanc romanc changed the title deps: get the right serialbox dependency (and make it optional) deps: remove usage of serialbox in pyFV3 Mar 9, 2026
@romanc
Copy link
Collaborator Author

romanc commented Mar 9, 2026

@oelbert I've added a commit to see what would be removed following @FlorianDeconinck's suggestion. Please review and give feedback as you see fit.

Copy link
Collaborator

@oelbert oelbert left a comment

Choose a reason for hiding this comment

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

You're right, we should strip Serialbox out of the dependencies for everything except NDSL at this point

@oelbert
Copy link
Collaborator

oelbert commented Mar 10, 2026

@FlorianDeconinck if you approve we can merge

@oelbert oelbert enabled auto-merge March 10, 2026 18:09
Copy link
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

J'approuve.

@oelbert oelbert added this pull request to the merge queue Mar 10, 2026
Merged via the queue into NOAA-GFDL:develop with commit 20b0007 Mar 10, 2026
5 checks passed
@romanc romanc deleted the romanc/serialbox-dependency branch March 11, 2026 08:48
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.

5 participants