Skip to content

[1pt] PR: Fix eval_plots for env file and misc cleanup #1580

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

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

RobHanna-NOAA
Copy link
Contributor

@RobHanna-NOAA RobHanna-NOAA commented Jun 27, 2025

Opportunistically updating eval_plots when running it as part of fim_performance production output, and not part of the synthesize_test_case usage of eval_plots. See closes 1202.

Misc cleanup

  • get rid of a gdal warning
  • Removing some old invalid or unused files (confirmed with team)

Changes

  • tools
    • eval_plots.py: Changed to look for explicit config env file via arg
    • pixel_counter.py: Added a line that stops a gdal warning of:

image

Removals

  • src
    • aggregate_fim_outputs.py
    • finalize_srcs.py
    • reset_mannings.py
  • tools
    • adjust_rc_with_feedback_py
  • .github\PULL_REQUEST_TEMPLATE.MD: Updates. This PR reflects those changes, mostly to DevOps questions.

Testing

Generally, you do not copy this part into the ChangeLog). These are some quick notes on what you did test or what the reviewer to know for their tests.

  • Ran eval_plots to get the FIM Performance files that are required for the FIM 6 production release. Outputs already validated, no more testing required.
  • Ran eval_plots against the metrics file in EFS / ouputs/hand_4_8_6_1_orig and it was fine.

Deployment Plan (For developer use)

  • Does the change impact inputs, docker or python packages?
    • Yes
    • No
  • Has new or updated python packages or Dockerfile changes?
    • Yes
  • Require new or adjusted data inputs? Does it have a way to version (folder or file dates?)
    • Yes
  • If new or updated data sets, has the FIM code been updated and tested with the new/adjusted data (subset is fine, but must be a subset of the new data)?
    • Yes
  • Require new pre-clip set?
    • Yes
  • If applicable, has a deployment plan be created with the DevOps team? including what was run already, what needs to be copied to all 4/5 enviros, from where, what still needs to be run at scale, etc. Any notes are helpful.
    • Yes
    • I will shortly. :)

Notes to DevOps Team:


Issuer Checklist (For developer use)

You may update this checklist before and/or after creating the PR. If you're unsure about any of them, please ask, we're here to help! These items are what we are going to look for before merging your code.

  • Informative and human-readable title, using the format: [_pt] PR: <description>
  • Links are provided if this PR resolves an issue, or depends on another other PR
  • If submitting a PR to the dev branch (the default branch), you have a descriptive Feature Branch name using the format: dev-<description-of-change> (e.g. dev-revise-levee-masking)
  • Changes are limited to a single goal (no scope creep)
  • The feature branch you're submitting as a PR is up to date (merged) with the latest dev branch
  • pre-commit hooks were run locally
  • 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
  • CHANGELOG updated with template version number, e.g. 4.x.x.x
  • Add yourself as an assignee in the PR as well as the FIM Technical Lead

Merge Checklist (For Technical Lead use only)

  • Update CHANGELOG with latest version number and merge date
  • Update the Citation.cff file to reflect the latest version number in the CHANGELOG
  • If applicable, update README with major alterations

@RobHanna-NOAA RobHanna-NOAA marked this pull request as ready for review June 30, 2025 19:42
@RobHanna-NOAA RobHanna-NOAA requested a review from mluck June 30, 2025 19:44
@RobHanna-NOAA RobHanna-NOAA changed the title WIP PR: Fix eval_plots for env file and misc cleanup [1pt] PR: Fix eval_plots for env file and misc cleanup Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[3pt] ENV file error: Change code calls using get_metadata to use input args to get that file name (ie. Eval_plots)
1 participant