Skip to content

Remove old combiner instances - #1000

Open
singhd789 wants to merge 12 commits into
NOAA-GFDL:mainfrom
singhd789:993.remove-old-combiner
Open

singhd789 wants to merge 12 commits into
NOAA-GFDL:mainfrom
singhd789:993.remove-old-combiner

Conversation

@singhd789

@singhd789 singhd789 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Describe your changes

  • Remove old yaml combiner, abstract_classes.py, info_parsers, join constructor
  • update instances where combiner is used/called

Needs this PR merged in first (for failing fre make tests): #999

Issue ticket number and link (if applicable)

Fixes #993

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

Note: If you are a code maintainer updating the tag or releasing a new fre-cli version, please use the release_procedure.md template. To quickly use this template, open a new pull request, choose your branch, and add ?template=release_procedure.md to the end of the url.

Copilot AI balanced review requested due to automatic review settings October 27, 2026 18:44
@singhd789 singhd789 changed the title #993 remove old combiner instances Remove old combiner instances Oct 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unmigrated consumers of removed imports, APIs, and YAML support cause runtime failures and prevent test collection.

4 open issues
What changed in this PR

Retires the legacy YAML combiner for issue #993 and makes the existing uwtools-based implementation the standard combiner used by FRE.

Changes:

  • Removes the legacy command, parser classes, abstract classes, and !join constructor.
  • Moves the replacement combiner to the canonical module name and updates list, post-processing, and test imports.
  • Switches the shared YAML loader to safe_load and removes obsolete documentation.
File Description
fre/​yamltools/​tests/​test_combine_yamls_script.py Updates the combiner import.
fre/​yamltools/​README.md Removes legacy command documentation.
fre/​yamltools/​info_parsers/​pp_info_parser.py Deletes the legacy post-processing parser.
fre/​yamltools/​info_parsers/​compile_info_parser.py Deletes the legacy compilation parser.
fre/​yamltools/​info_parsers/​analysis_info_parser.py Deletes the legacy analysis parser.
fre/​yamltools/​helpers.py Uses safe loading and removes experiment checking.
fre/​yamltools/​freyamltools.py Removes the legacy command and redirects combine.
fre/​yamltools/​constructors.py Deletes the !join constructor.
fre/​yamltools/​combine_yamls_script.py Replaces the legacy implementation with the uwtools combiner.
fre/​yamltools/​combine_yamls_script_new.py Removes the former replacement-module location.
fre/​yamltools/​abstract_classes.py Deletes legacy merging interfaces.
fre/​yamltools/​__init__.py Disables constructor registration and the YAML import.
fre/​pp/​tests/​test_configure_script_yaml.py Updates the test combiner import.
fre/​pp/​configure_script_yaml.py Updates the post-processing combiner import.
fre/​list_/​list_pp_components_script.py Updates the component-listing combiner import.
fre/​list_/​list_platforms_script.py Updates the platform-listing combiner import.
fre/​list_/​list_experiments_script.py Updates the experiment-listing combiner import.

Review effort: Balanced

Comment thread fre/yamltools/__init__.py
Comment thread fre/yamltools/combine_yamls_script.py Outdated
Comment thread fre/yamltools/helpers.py
Comment thread fre/yamltools/helpers.py
@singhd789
singhd789 marked this pull request as ready for review October 28, 2026 19:33

This branch has not been deployed

No deployments
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.

Remove old combine-yamls subtool

2 participants