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

Omnibus 2024-11-27 #663

Merged
merged 34 commits into from
Dec 4, 2024
Merged

Conversation

maddenp-noaa
Copy link
Collaborator

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

Synopsis

  • Update .github such that the Test workflow will be run for PRs or pushes to all branches. This means that tests will run for updates on release branches, which seems like it would have been a good idea to always be doing.
  • Add setuptools to conda build requirements, as it appears not to be automatically installed with Python 3.13 now in CI jobs, but is required for the uwtools build.
  • Some updated documentation about the !include tag.
  • Some trivial doc changes per make docs.
  • Fix a bug whereby Jinja2 expressions referencing tagged YAML values would render to strings including the tag as part of the string. The fix involves detecting tagged values and raising an exception to delay rendering the value until a later iteration of the dereferencing process, when the tagged value will have been converted to its simple type (int, bool, etc.). See below (and the new unit tests) for examples.

Given config.yaml

a: 2
b: 7
foo: !int "{{ a * b }}"
bar: !int "{{ foo }}"

Old behavior:

$ uw config realize -i config.yaml --output-format yaml
a: 2
b: 7
foo: 14
bar: !int '!int 14'

New behavior:

$ uw config realize -i config.yaml --output-format yaml
a: 2
b: 7
foo: 14
bar: 14

Type

  • Bug fix (corrects a known issue)
  • Documentation
  • Tooling (CI, code-quality, packaging, revision-control, etc.)

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 maddenp-noaa self-assigned this Nov 27, 2024
src/uwtools/config/jinja2.py Show resolved Hide resolved
src/uwtools/config/jinja2.py Outdated Show resolved Hide resolved
src/uwtools/tests/config/test_tools.py Show resolved Hide resolved
src/uwtools/config/formats/yaml.py Show resolved Hide resolved
src/uwtools/config/jinja2.py Show resolved Hide resolved
@maddenp-noaa maddenp-noaa changed the title Omnibus 2024-11-2 Omnibus 2024-11-27 Nov 27, 2024
@maddenp-noaa maddenp-noaa marked this pull request as ready for review November 27, 2024 06:47
docs/sections/user_guide/yaml/tags.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@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. This makes for a much cleaner user experience than my workaround.

Maybe a small docs change is needed?

src/uwtools/config/formats/yaml.py Show resolved Hide resolved
docs/sections/user_guide/yaml/tags.rst Show resolved Hide resolved
@maddenp-noaa maddenp-noaa merged commit fe832c1 into ufs-community:main Dec 4, 2024
2 checks passed
@maddenp-noaa maddenp-noaa deleted the double-tag-fix branch December 4, 2024 19:44
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.

5 participants