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.

Sign up for GitHub

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

Jump to bottom

UW-608 filter_topo driver #510

Merged
merged 44 commits into from
Jun 17, 2024
Merged

UW-608 filter_topo driver #510

merged 44 commits into from
Jun 17, 2024

Conversation

maddenp-noaa
Copy link
Contributor

@maddenp-noaa maddenp-noaa commented Jun 14, 2024 •
edited
Loading

Synopsis

A driver for the filter_topo utility. A sample run on hera: /home/Paul.Madden/git/uwtools/run.filter_topo, configured by /home/Paul.Madden/git/uwtools/filter_topo.yaml.

I also noticed some repeated patterns in the drivers and their unit tests and took the opportunity to do some refactoring:

  1. Many drivers implemented an identical, simple runscript() method, so I factored that out to the Driver base class and let them inherit it. Several drivers override this default with a custom method.
  2. Many of the driver unit tests repeatedly re-test methods from the Driver base class, which have their own unit tests. So I DRYed out those tests and replaced them with a test that asserts that certain methods on a derived class are, in fact, identical to methods in the base class -- which we know work correctly because they have their own tests.

I also tidied up the docs in a few places.

Type

  • Code maintenance (refactoring, etc. without behavior change)
  • Documentation
  • Enhancement (adds new functionality)

Impact

  • This is a non-breaking change (existing functionality continues to work as expected)

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
Squash-merge UW-607 mods
cefda82
@maddenp-noaa
WIP
5694e59
@maddenp-noaa
WIP
fdc1ea2
@maddenp-noaa
WIP
01117c7
@maddenp-noaa
WIP
720606c
@maddenp-noaa
WIP
da624b2
@maddenp-noaa
WIP
4da9930
@maddenp-noaa
Squashed commit of the following:
6df994a 
commit d5d9efa
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 17:44:35 2024 +0000

    Update test_driver.py to test config file case

commit 4998205
Merge: 5b9cd8c dbe1224
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 14:51:36 2024 +0000

    Merge branch 'main' into uw-607-orog-gsl-driver

commit 5b9cd8c
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 14:18:46 2024 +0000

    Update doc

commit 6c289f2
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 13:54:07 2024 +0000

    Fix section headers in test_schemas.py

commit 129d748
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 04:17:56 2024 +0000

    Update make_hgrid unit test

commit 7ce5507
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 03:59:13 2024 +0000

    Fix docs typo

commit 38a6d92
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 01:42:27 2024 +0000

    Work on docs

commit b2b9818
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 01:25:19 2024 +0000

    Add orog_gsl UW YAML doc

commit ddb0ad1
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Thu Jun 13 01:19:12 2024 +0000

    WIP

commit d4062c5
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 21:43:42 2024 +0000

    Work on docs

commit 564f9f6
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 21:38:24 2024 +0000

    Unit tests @ 100%

commit 72cd854
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 21:35:21 2024 +0000

    Work on unit tests

commit 47e22ea
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 20:49:29 2024 +0000

    Use in-memory configs in driver tests to reduce IO

commit c9e1229
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 20:33:33 2024 +0000

    Use in-memory configs in driver tests to reduce IO

commit 918f1fd
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 20:12:59 2024 +0000

    Work on unit tests

commit 308bdc3
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 20:05:53 2024 +0000

    Add API unit tests

commit d424b78
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 20:05:45 2024 +0000

    Add module envcmds to prototype YAML config

commit 3873d36
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 20:02:13 2024 +0000

    Add schema tests

commit b2ef311
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 19:41:32 2024 +0000

    Delinting

commit da4c4ac
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 19:34:36 2024 +0000

    WIP

commit 1dc8206
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 19:25:35 2024 +0000

    Reorder required in shave.jsonschema

commit 7650959
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 19:23:40 2024 +0000

    Add orog-gsl.jsonschema

commit cf6db0c
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 19:15:17 2024 +0000

    Work on driver

commit 2867918
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 16:45:09 2024 +0000

    Add orog_gsl.py

commit 17a0eaf
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 16:44:57 2024 +0000

    Simplify Driver._write_runscript() signature and callers

commit 3758ed1
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 16:38:13 2024 +0000

    Tidy

commit 5f6b951
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 16:36:57 2024 +0000

    Update CLI and API

commit 2568762
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 16:35:55 2024 +0000

    Update prototype YAML config

commit 9773df5
Author: Paul Madden <paul.madden@noaa.gov>
Date:   Wed Jun 12 15:49:58 2024 +0000

    Add prototype orog_gsl.yaml
@maddenp-noaa
Merge branch 'main' into uw-608-filter-topo-driver
fa79f28
@maddenp-noaa
Finish schema tests
1caf5ee
@maddenp-noaa
WIP on tests
dc45f67
@maddenp-noaa
WIP on tests
f2b06e6
@maddenp-noaa
WIP on tests
aa239eb
@maddenp-noaa
WIP on tests
4884926
@maddenp-noaa
WIP on tests
20e8cd0
@maddenp-noaa
Simplify
7631c20
@maddenp-noaa
Simplify
c34372e
@maddenp-noaa
Simplify
26d9225
@maddenp-noaa
Simplify
84a6188
@maddenp-noaa
Simplify
1f18419
@maddenp-noaa
Simplify
b3bc0e4
@maddenp-noaa
Simplify
2d052c3
@maddenp-noaa
Simplify
da96104
@maddenp-noaa
Simplify
bb85d49
@maddenp-noaa
_taskname() is also inhertied
6c024cc
@maddenp-noaa
Simplify
ee8b9d0
@maddenp-noaa
Simplify
abfbb35
@maddenp-noaa
Simplify
06599c6
@maddenp-noaa
Simplify
61af1fa
@maddenp-noaa
Simplify
eb8bd40
@maddenp-noaa maddenp-noaa self-assigned this Jun 14, 2024
maddenp-noaa
maddenp-noaa commented Jun 14, 2024
View reviewed changes
docs/sections/user_guide/yaml/components/filter_topo.rst Show resolved Hide resolved
src/uwtools/config/support.py Show resolved Hide resolved
src/uwtools/drivers/driver.py Show resolved Hide resolved
src/uwtools/drivers/upp.py Show resolved Hide resolved
src/uwtools/tests/config/test_support.py Show resolved Hide resolved
src/uwtools/tests/drivers/test_chgres_cube.py Show resolved Hide resolved
src/uwtools/tests/drivers/test_schism.py Show resolved Hide resolved
@maddenp-noaa maddenp-noaa marked this pull request as ready for review June 14, 2024 15:34
maddenp-noaa and others added 2 commits June 14, 2024 09:53
@maddenp-noaa @elcarpenterNOAA
Update src/uwtools/config/support.py
1f42edd 
Co-authored-by: Emily Carpenter <137525341+elcarpenterNOAA@users.noreply.github.com>
@maddenp-noaa
Merge branch 'uw-608-filter-topo-driver' of github.com:maddenp-noaa/u…
bfa3058 
…wtools into uw-608-filter-topo-driver
WeirAE
WeirAE approved these changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@WeirAE WeirAE left a comment

Choose a reason for hiding this comment

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

Solid driver, great refactor - thanks!

christinaholtNOAA
christinaholtNOAA reviewed Jun 14, 2024
View reviewed changes
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.

This looks great.

I have a future question inspired by this test clean up work. Can we have a test that fails if we haven't tested all the methods on a driver? I was thinking something like checking that the set(attributes of Driver subclass - attributes of Driver class) show up in the tests in the driver test file.

docs/sections/user_guide/cli/drivers/filter_topo.rst Outdated Show resolved Hide resolved
docs/sections/user_guide/yaml/components/filter_topo.rst Show resolved Hide resolved
docs/sections/user_guide/yaml/components/sfc_climo_gen.rst Show resolved Hide resolved
docs/shared/filter_topo.yaml Show resolved Hide resolved
src/uwtools/drivers/global_equiv_resol.py Show resolved Hide resolved
src/uwtools/tests/drivers/test_filter_topo.py Show resolved Hide resolved
src/uwtools/tests/drivers/test_jedi.py Show resolved Hide resolved
src/uwtools/tests/drivers/test_jedi.py Outdated Show resolved Hide resolved
maddenp-noaa and others added 3 commits June 14, 2024 13:04
@maddenp-noaa @christinaholtNOAA
Update docs/sections/user_guide/cli/drivers/filter_topo.rst
bf27203 
Co-authored-by: Christina Holt <56881914+christinaholtNOAA@users.noreply.github.com>
@maddenp-noaa @christinaholtNOAA
Update src/uwtools/tests/drivers/test_jedi.py
2cfbe89 
Co-authored-by: Christina Holt <56881914+christinaholtNOAA@users.noreply.github.com>
@maddenp-noaa
Remove too-specific runscript command-line content from docs
100d81d
christinaholtNOAA
christinaholtNOAA reviewed Jun 17, 2024
View reviewed changes
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.

LGTM.

elcarpenterNOAA
elcarpenterNOAA approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@elcarpenterNOAA elcarpenterNOAA left a comment

Choose a reason for hiding this comment

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

Looks good for me. Thanks for making all the additional updates.

@maddenp-noaa maddenp-noaa merged commit 7646f58 into ufs-community:main Jun 17, 2024
2 checks passed
@maddenp-noaa maddenp-noaa deleted the uw-608-filter-topo-driver branch June 17, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christinaholtNOAA christinaholtNOAA christinaholtNOAA approved these changes

@WeirAE WeirAE WeirAE approved these changes

@elcarpenterNOAA elcarpenterNOAA elcarpenterNOAA approved these changes

@jprestop jprestop Awaiting requested review from jprestop jprestop is a code owner

@NaureenBharwaniNOAA NaureenBharwaniNOAA Awaiting requested review from NaureenBharwaniNOAA NaureenBharwaniNOAA is a code owner

@fgabelmannjr fgabelmannjr Awaiting requested review from fgabelmannjr fgabelmannjr is a code owner

Assignees

@maddenp-noaa maddenp-noaa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@maddenp-noaa @christinaholtNOAA @WeirAE @elcarpenterNOAA