fre pp rename-split #717

ceblanton · 2026-02-03T03:46:05Z

Describe your changes

Conversion of fre-workflows rename-split shell script into fre-cli python. Preserved existing rename-split test cases.

Issue ticket number and link (if applicable)

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

…me_split_to_pp.py

The ending date should be June 30

from fre-cli instead of fre-workflows

codecov · 2026-02-03T05:42:29Z

Codecov Report

❌ Patch coverage is 89.23077% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.94%. Comparing base (b34af59) to head (ab3ba21).

Files with missing lines	Patch %	Lines
fre/pp/rename_split_script.py	89.34%	13 Missing ⚠️
fre/pp/frepp.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #717      +/-   ##
==========================================
+ Coverage   82.76%   82.94%   +0.18%     
==========================================
  Files          68       69       +1     
  Lines        4525     4655     +130     
==========================================
+ Hits         3745     3861     +116     
- Misses        780      794      +14

Flag	Coverage Δ
unittests	`82.94% <89.23%> (+0.18%)`	⬆️

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

Files with missing lines	Coverage Δ
fre/pp/frepp.py	`86.55% <87.50%> (+0.06%)`	⬆️
fre/pp/rename_split_script.py	`89.34% <89.34%> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b34af59...ab3ba21. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ilaflott · 2026-02-04T16:20:50Z

fre/pp/tests/test_rename_split_to_pp.py

+    nc_exists = [osp.isfile(el) for el in nc_files]
+    assert all(nc_exists)
+
+def test_rename_split_to_pp_multiply():


this is a setup function too

ilaflott · 2026-02-04T16:25:48Z

@copilot open a PR to this branch and:

try reducing the cdl files to a minimal set of required data points
the unit tests must succeed
try with just the atmos_daily case(s) first

Copilot · 2026-02-04T16:25:58Z

@ilaflott I've opened a new pull request, #719, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>

reduce `rename-split` test-data to minimal possible size

ilaflott

a bunch of this is nit picking about style, this looks largely functional. i'll ponder approving when it's taken out of draft

ilaflott · 2026-02-04T17:34:47Z

fre/pp/tests/test_rename_split_to_pp.py

+#tests rename_split_to_pp with a minimal python wrapper
+#test cases: regrid/native, static/ts
+#fail test cases: no files in your input directory/subdirs; input files are not named according to our very specific naming conventions


lets make this a proper docstring

ilaflott · 2026-02-04T17:35:03Z

fre/pp/tests/test_rename_split_to_pp.py

+#https://stackoverflow.com/questions/67631/how-can-i-import-a-module-dynamically-given-the-full-path
+#/archive/cew/CMIP7/ESM4/DEV/ESM4.5_staticfix/ppan-prod-openmp/history/00010101.nc.tar


can remove i think

ilaflott · 2026-02-04T17:35:27Z

fre/pp/tests/test_rename_split_to_pp.py

+import pprint
+from pathlib import Path
+import cftime
+from fre.pp.rename_split_script import *


generally disfavor wildcard imports, if possible.

ilaflott · 2026-02-04T17:36:43Z

fre/pp/tests/test_rename_split_to_pp.py

general spacing consistency- two spaces or four? the rest of the repo uses 4

ilaflott · 2026-02-04T17:39:40Z

fre/pp/tests/test_rename_split_to_pp.py

+import pytest
+import sys
+import os
+from os import path as osp
+import subprocess
+import re
+import pprint
+from pathlib import Path
+import cftime
+from fre.pp.rename_split_script import *


generally, import stdlib modules first, third-party second, then internal dependencies last. some additionally sort by alphabetical order but this is more matter of taste imo

Chris Blanton and others added 8 commits January 23, 2026 17:36

Initial work to move rename-split-to-pp from fre-workflows

b30685a

Direct import of fre-workflows/app/rename-split-to-pp/tests/test_rena…

c59c9e5

…me_split_to_pp.py

Direct import of existing fre-workflows rename-split test files

2f22e51

Fixed input test files for daily frequency 6-month chunk.

173974d

The ending date should be June 30

Minor updates to original rename-split test to run rename-split

6f4637c

from fre-cli instead of fre-workflows

Adjusted rename-split to work for almost all existing test cases

0cfcbe3

Added rename-split functionality for one timestep with time bounds case

7c31c7d

cosmetic

63274d2

Chris Blanton and others added 3 commits February 3, 2026 01:19

add tests for rename-split.get_freq_and_format_from_two_dates

fca4348

add rename-split sub-tests for get_duration_from_two_dates

d62e5a9

Merge branch 'main' into rename-split

4ae680b

ilaflott self-requested a review February 4, 2026 15:10

ilaflott reviewed Feb 4, 2026

View reviewed changes

Initial plan

c5c831e

Copilot AI mentioned this pull request Feb 4, 2026

reduce rename-split test-data to minimal possible size #719

Merged

8 tasks

Copilot AI and others added 6 commits February 4, 2026 16:36

Reduce CDL files to minimal dataset and atmos_daily only

3ea0819

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>

Restore non-atmos_daily test files while keeping atmos_daily minimal

a0aef8e

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>

Add 'rename-split' branch to workflow triggers

87c5f3a

Reduce ocean_annual test files to minimal size

b3300ce

Co-authored-by: ilaflott <6273252+ilaflott@users.noreply.github.com>

Remove 'rename-split' branch from workflow triggers

d74b487

Merge pull request #719 from NOAA-GFDL/copilot/sub-pr-717

ab3ba21

reduce `rename-split` test-data to minimal possible size

ilaflott reviewed Feb 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fre pp rename-split #717

fre pp rename-split #717

Uh oh!

ceblanton commented Feb 3, 2026

Uh oh!

codecov bot commented Feb 3, 2026 •

edited

Loading

Uh oh!

ilaflott Feb 4, 2026

Uh oh!

ilaflott commented Feb 4, 2026

Uh oh!

Copilot AI commented Feb 4, 2026

Uh oh!

ilaflott left a comment

Uh oh!

ilaflott Feb 4, 2026

Uh oh!

ilaflott Feb 4, 2026

Uh oh!

ilaflott Feb 4, 2026

Uh oh!

ilaflott Feb 4, 2026

Uh oh!

ilaflott Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		#https://stackoverflow.com/questions/67631/how-can-i-import-a-module-dynamically-given-the-full-path
		#/archive/cew/CMIP7/ESM4/DEV/ESM4.5_staticfix/ppan-prod-openmp/history/00010101.nc.tar

fre pp rename-split #717

Are you sure you want to change the base?

fre pp rename-split #717

Uh oh!

Conversation

ceblanton commented Feb 3, 2026

Describe your changes

Issue ticket number and link (if applicable)

Checklist before requesting a review

Uh oh!

codecov bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ilaflott Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

ilaflott commented Feb 4, 2026

Uh oh!

Copilot AI commented Feb 4, 2026

Uh oh!

ilaflott left a comment

Choose a reason for hiding this comment

Uh oh!

ilaflott Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

ilaflott Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

ilaflott Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

ilaflott Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

ilaflott Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Feb 3, 2026 •

edited

Loading