Skip to content
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

Revamp config compare for compatibility with more configs #654

Merged
merged 26 commits into from
Nov 15, 2024

Conversation

maddenp-noaa
Copy link
Contributor

@maddenp-noaa maddenp-noaa commented Nov 14, 2024

Synopsis

Fixes #622.

In addition to fixing the compare-two-identical-.sh-configs example from the ticket, .sh configs with differences are now handled, with differences shown using a familiar diff syntax:

a.sh

foo=bar
bar=banana
baz=2

b.sh

foo=bar
bar=papaya
$ uw config compare --file-1-path a.sh --file-2-path b.sh
[2024-11-14T23:47:32]     INFO - a.sh
[2024-11-14T23:47:32]     INFO + b.sh
[2024-11-14T23:47:32]     INFO ---------------------------------------------------------------------
[2024-11-14T23:47:32]     INFO ↓ ? = info | -/+ = line unique to - or + file | blank = matching line
[2024-11-14T23:47:32]     INFO ---------------------------------------------------------------------
[2024-11-14T23:47:32]     INFO - bar: banana
[2024-11-14T23:47:32]     INFO ?      ^ ^ ^
[2024-11-14T23:47:32]     INFO + bar: papaya
[2024-11-14T23:47:32]     INFO ?      ^ ^ ^
[2024-11-14T23:47:32]     INFO - baz: '2'
[2024-11-14T23:47:32]     INFO   foo: bar

Also, depth > 2 configs, which also were not previously processed, now work:

a.yaml

foo:
  bar:
    baz: qux

b.yaml

foo:
  bar:
    baz: asdf
$ uw config compare --file-1-path a.yaml --file-2-path b.yaml
[2024-11-14T23:50:03]     INFO - a.yaml
[2024-11-14T23:50:03]     INFO + b.yaml
[2024-11-14T23:50:03]     INFO ---------------------------------------------------------------------
[2024-11-14T23:50:03]     INFO ↓ ? = info | -/+ = line unique to - or + file | blank = matching line
[2024-11-14T23:50:03]     INFO ---------------------------------------------------------------------
[2024-11-14T23:50:03]     INFO   foo:
[2024-11-14T23:50:03]     INFO     bar:
[2024-11-14T23:50:03]     INFO -     baz: qux
[2024-11-14T23:50:03]     INFO +     baz: asdf

Mixed-depth configs work:
c.yaml

foo:
  bar: qwerty
$ uw config compare --file-1-path a.yaml --file-2-path c.yaml
[2024-11-14T23:51:55]     INFO - a.yaml
[2024-11-14T23:51:55]     INFO + c.yaml
[2024-11-14T23:51:55]     INFO ---------------------------------------------------------------------
[2024-11-14T23:51:55]     INFO ↓ ? = info | -/+ = line unique to - or + file | blank = matching line
[2024-11-14T23:51:55]     INFO ---------------------------------------------------------------------
[2024-11-14T23:51:55]     INFO   foo:
[2024-11-14T23:51:55]     INFO +   bar: qwerty
[2024-11-14T23:51:55]     INFO -   bar:
[2024-11-14T23:51:55]     INFO -     baz: qux

Namelist configs are compared via their YAML representations:
a.nml

&a s="foo" n=999 /

b.nml

&a
  s="foo"
  n=1
/
$ uw config compare --file-1-path a.nml --file-2-path b.nml
[2024-11-14T23:56:13]     INFO - a.nml
[2024-11-14T23:56:13]     INFO + b.nml
[2024-11-14T23:56:13]     INFO ---------------------------------------------------------------------
[2024-11-14T23:56:13]     INFO ↓ ? = info | -/+ = line unique to - or + file | blank = matching line
[2024-11-14T23:56:13]     INFO ---------------------------------------------------------------------
[2024-11-14T23:56:13]     INFO   a:
[2024-11-14T23:56:13]     INFO -   n: 999
[2024-11-14T23:56:13]     INFO ?      ^^^
[2024-11-14T23:56:13]     INFO +   n: 1
[2024-11-14T23:56:13]     INFO ?      ^
[2024-11-14T23:56:13]     INFO     s: foo

Unit tests are updated/expanded, and docs are updated.

This may be considered a breaking change because the logged output is different.

Type

  • Bug fix (corrects a known issue)
  • Documentation
  • Enhancement (adds new functionality)

Impact

  • This is a breaking change (changes existing functionality)

Checklist

  • I have added myself and any co-authors to the PR's Assignees list.
  • I have reviewed the documentation and have made any updates necessitated by this change.

@maddenp-noaa maddenp-noaa self-assigned this Nov 14, 2024
Copy link
Contributor

@christinaholtNOAA christinaholtNOAA left a comment

Choose a reason for hiding this comment

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

Geez! You really went above and beyond on the bug fix. The user experience upgrade and the clean solution are fantastic!

src/uwtools/tests/config/formats/test_base.py Show resolved Hide resolved
src/uwtools/config/formats/base.py Show resolved Hide resolved
@maddenp-noaa maddenp-noaa merged commit db80f2c into ufs-community:main Nov 15, 2024
2 checks passed
@maddenp-noaa maddenp-noaa deleted the gh622-sh-compare-fix branch November 15, 2024 18:40
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.

Comparing two sh configs is broken.
2 participants