Skip to content

Conversation

mlee03
Copy link
Contributor

@mlee03 mlee03 commented Aug 25, 2025

Describe your changes

This PR restores the original directory structure where

  1. work_dir is the working directory containing the extracted grid_spec.nc, mosaic, and grid files from the grid_spec tarfile.
  2. remap_dir is the directory containing the remap files that are either generated or reused during the regridding workflow.

These directories remove the need to move and copy relevant files before fregrid is invoked.

This PR still requires changes to fre-workflow and requires testing within the workflow

Issue ticket number and link (if applicable)

#609

Checklist before requesting a review

  • I ran my code
  • I tried to make my code readable
  • I tried to comment my code
  • I wrote a new test, if applicable
  • I wrote new instructions/documentation, if applicable
  • I ran pytest and inspected it's output
  • I ran pylint and attempted to implement some of it's feedback
  • No print statements; all user-facing info uses logging module

Copy link

codecov bot commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 91.22807% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.74%. Comparing base (b287d62) to head (7d8d148).
⚠️ Report is 204 commits behind head on main.

Files with missing lines Patch % Lines
fre/app/regrid_xy/regrid_xy.py 90.38% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #617      +/-   ##
==========================================
+ Coverage   73.67%   73.74%   +0.06%     
==========================================
  Files          65       65              
  Lines        4175     4178       +3     
==========================================
+ Hits         3076     3081       +5     
+ Misses       1099     1097       -2     
Flag Coverage Δ
unittests 73.74% <91.22%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mlee03 mlee03 marked this pull request as ready for review August 27, 2025 13:05
@ilaflott ilaflott linked an issue Aug 27, 2025 that may be closed by this pull request
Copy link
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

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

largely looks good, but i'm concerned the os.chdir isn't as transient as we need it to be.

@mlee03
Copy link
Contributor Author

mlee03 commented Aug 27, 2025

Remember to finish reviewing this PR 😁

Copy link
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

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

I know @mlee03 appreciates a thorough review, so i took more time to be more thorough!

@mlee03
Copy link
Contributor Author

mlee03 commented Aug 28, 2025

additional change that will come, the f-strings will be changed to use %s for the logger

@ilaflott ilaflott requested a review from singhd789 September 12, 2025 21:02
Copy link
Collaborator

@singhd789 singhd789 left a comment

Choose a reason for hiding this comment

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

All my comments look like they're addressed and the tests pass! Wooo, good job. I'd just say merge in main and we're good to go.

@ilaflott ilaflott merged commit 7a27ea3 into NOAA-GFDL:main Sep 12, 2025
6 checks passed
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.

bug in the new regrid_xy
4 participants