Skip to content

Geo test pre refactor#125

Open
mdeshotel wants to merge 33 commits intomdeshotel_pre_devfrom
geo_test_pre_refactor
Open

Geo test pre refactor#125
mdeshotel wants to merge 33 commits intomdeshotel_pre_devfrom
geo_test_pre_refactor

Conversation

@mdeshotel
Copy link
Copy Markdown

This PR sets up tests for GeoMeta and InputForcings classes prior to refactoring. The intent is to establish pre-refactored results for comparison to post-refactored results.

Additions

  • Tests for GeoMeta and InputForcings classes

Removals

Changes

Testing

  1. Tests for GeoMeta and InputForcings classes appear to be working as expected for pre-refactored code.

Screenshots

Notes

Todos

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Linux

@mxkpp mxkpp self-requested a review March 27, 2026 16:17
Copy link
Copy Markdown

@mxkpp mxkpp left a comment

Choose a reason for hiding this comment

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

I built using these versions and got the geomod n1 test to pass as is without modifying the current expected test data.

(with geo_test_pre_refactor checked out locally for ngen-forcing):

REPO_TAG_FCST_MGR="856fc0e1201076df909e56c7cd384f58e82965a2"
REPO_TAG_MSW_MGR="693c206a22b5e9ffcca3103166c0ca59e2b11b25"
REPO_TAG_CAL_MGR="7e56bf01477ea77e72dfb25a166ac26ff6090ecb"
REPO_TAG_NGEN_FORCING="LOCAL"
NGEN_SOURCE_MODE="ghcr"
NGEN_BASE__REMOTE_GHCR_TAG="844c5f6"

Please see initial requested changes.

@mdeshotel mdeshotel requested a review from mxkpp March 28, 2026 17:16
Copy link
Copy Markdown

@mxkpp mxkpp left a comment

Choose a reason for hiding this comment

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

Please see below

@mxkpp
Copy link
Copy Markdown

mxkpp commented Mar 30, 2026

Can these files be removed?

test_expected_geomod_after_update_step_1.json
test_expected_geomod_after_update_step_2.json
test_expected_geomod_after_update_step_3.json
test_expected_geomod_finalize.json
test_expected_geomod_init.json
test_expected_input_forcing_after_update_step_1.json
test_expected_input_forcing_after_update_step_2.json
test_expected_input_forcing_after_update_step_3.json
test_expected_input_forcing_finalize.json
test_expected_input_forcing_init.json

@mdeshotel mdeshotel requested a review from mxkpp March 31, 2026 18:34
Copy link
Copy Markdown

@mxkpp mxkpp left a comment

Choose a reason for hiding this comment

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

All of the original regrid test data should remain, those should not be deleted. For example these n1 files and their corresponding n2s:

tests/test_data/expected_results/test_expect_regrid_aorc_aws__gauge_01123000_start20130701000000_n1_rank0_timestep0.json
tests/test_data/expected_results/test_expect_regrid_conus_hrrr__gauge_01123000_start20250710040000_n1_rank0_timestep0.json
tests/test_data/expected_results/test_expect_regrid_conus_rap__gauge_01123000_start20250710040000_n1_rank0_timestep0.json

@mdeshotel
Copy link
Copy Markdown
Author

All of the original regrid test data should remain, those should not be deleted. For example these n1 files and their corresponding n2s:

tests/test_data/expected_results/test_expect_regrid_aorc_aws__gauge_01123000_start20130701000000_n1_rank0_timestep0.json
tests/test_data/expected_results/test_expect_regrid_conus_hrrr__gauge_01123000_start20250710040000_n1_rank0_timestep0.json
tests/test_data/expected_results/test_expect_regrid_conus_rap__gauge_01123000_start20250710040000_n1_rank0_timestep0.json

This should be fixed now.

@mdeshotel mdeshotel requested a review from mxkpp April 3, 2026 16:31
Copy link
Copy Markdown

@mxkpp mxkpp left a comment

Choose a reason for hiding this comment

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

The np.full calls are missing dtype in this branch. I think once those are re-added, this is ready to merge to mdeshotel_pre_dev.

@mxkpp mxkpp self-requested a review April 5, 2026 13:57
Copy link
Copy Markdown

@mxkpp mxkpp 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. I re-created expected test results data to confirm that the committed test files don't change, and then I re-ran the tests.

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.

2 participants