Add standard names to romsds so decode_vertical_coords works as documented#650
Open
gaoflow wants to merge 1 commit into
Open
Add standard names to romsds so decode_vertical_coords works as documented#650gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
The bundled romsds dataset's depth (h) and eta (zeta) variables had no standard_name, so decode_vertical_coords could not derive the computed standard name and raised 'The standard name for the depth variable is not available' - including in the documented example. Add the standard names (matching what the existing test set inline) so the dataset is usable as documented, and drop the now-redundant inline assignments from the test. Fixes xarray-contrib#645. Signed-off-by: gaoflow <gaobing1230@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #645.
Problem
decode_vertical_coordsraises on the bundledromsdsdataset, including in the documented example:romsds.s_rhodeclaresformula_terms = "... eta: zeta depth: h ...", but the bundledh(depth) andzeta(eta) coordinates carry nostandard_name._derive_ocean_stdnameneeds those to look up the computed standard name, so it fails. Tellingly, the existingtest_decode_vertical_coordsworked around this by assigning the twostandard_names inline before callingdecode_vertical_coords— i.e. the bundled dataset was incomplete.Fix
Give the bundled
romsdshandzetathe standard names they need (sea_floor_depth_below_geopotential_datumandsea_surface_height_above_geopotential_datum— the same values the test was setting inline), so the dataset works as documented out of the box.The now-redundant inline assignments are dropped from
test_decode_vertical_coords, so the test exercises the bundled dataset directly and will catch a regression of this kind.decode_vertical_coordsnow yieldsz_rhowith the computed standard nameheight_above_geopotential_datum.Testing
test_parametric.pyandtest_accessor.pypass (205 passed, 24 skipped). The decode test fails without this change (the originalValueError) and passes with it.ruff checkandruff formatare clean.This change was prepared by an AI agent under my direction; I reviewed and verified it.