Skip to content

use yaml constructors for _IgnoreCustomTagsLoader#1907

Merged
braingram merged 3 commits intoasdf-format:mainfrom
braingram:load_yaml_tagged
Apr 3, 2025
Merged

use yaml constructors for _IgnoreCustomTagsLoader#1907
braingram merged 3 commits intoasdf-format:mainfrom
braingram:load_yaml_tagged

Conversation

@braingram
Copy link
Copy Markdown
Contributor

@braingram braingram commented Mar 12, 2025

Closes #1821

Description

The fix in #1825 was partial (only covering non-tagged nodes) since construct_mapping is not a generator but construct_yaml_map is).

This PR updates _IgnoreCustomTagsLoader to use construct_yaml_map and construct_yaml_seq (both generators) to handled tagged recursive structures.

Tasks

  • run pre-commit on your machine
  • run pytest on your machine
  • Does this PR add new features and / or change user-facing code / API? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • update relevant docstrings and / or docs/ page
    • for any new features, add unit tests
news fragment change types...
  • changes/<PR#>.feature.rst: new feature
  • changes/<PR#>.bugfix.rst: bug fix
  • changes/<PR#>.doc.rst: documentation change
  • changes/<PR#>.removal.rst: deprecation or removal of public API
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change

@braingram braingram marked this pull request as ready for review March 12, 2025 17:55
@braingram braingram requested a review from a team as a code owner March 12, 2025 17:55
@braingram
Copy link
Copy Markdown
Contributor Author

@emolter does this fix the error for you? I tested with the file you shared and with this PR it can be loaded with load_yaml.

@braingram
Copy link
Copy Markdown
Contributor Author

braingram commented Mar 12, 2025

Romancal regtests (as it uses load_yaml with tagged=False): https://github.com/spacetelescope/RegressionTests/actions/runs/13818535156 all pass

@emolter
Copy link
Copy Markdown

emolter commented Mar 12, 2025

Yes this does fix the error for me, thanks Brett

Copy link
Copy Markdown

@perrygreenfield perrygreenfield left a comment

Choose a reason for hiding this comment

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

LGTM

@braingram braingram merged commit 291528b into asdf-format:main Apr 3, 2025
50 checks passed
@braingram braingram deleted the load_yaml_tagged branch April 3, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

asdf.util.load_yaml fails on recursive object

3 participants