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

fix __asdf_traverse__ for non-tagged object #1739

Merged
merged 4 commits into from
May 7, 2024

Conversation

braingram
Copy link
Contributor

@braingram braingram commented Jan 23, 2024

Description

This PR adds a check for a _tag attribute before attempting to fetch schema for a node during NodeSchemaInfo tree traversal.

The current code assumes that any object implementing __asdf_traverse__ also has a _tag attribute which contains a valid tag.

"duck typing" was used here (instead of checking for isinstance(Tagged) as roman_datamodels defines non-Tagged objects that contain valid _tag attributes.

Fixes #1738

The weldx failure is unrelated (caused by a new pandas version) the roman_datamodels failure is addressed in: #1748

Checklist:

  • pre-commit checks ran successfully
  • tests ran successfully
  • for a public change, a changelog entry was added
  • for a public change, documentation was updated
  • for any new features, unit tests were added

@braingram
Copy link
Contributor Author

A few unrelated errors are seen in downstream:

  • the weldx errors are due to a new pandas version
  • the [specutils error] is due to a failure to fetch iers data
  • the [stdatamodels error] is due to a crds update

@braingram braingram marked this pull request as ready for review February 3, 2024 15:05
@braingram braingram requested a review from a team as a code owner February 3, 2024 15:05
@braingram braingram force-pushed the asdf_traverse branch 2 times, most recently from 88fee70 to 6a95536 Compare February 27, 2024 14:59
@braingram braingram modified the milestones: 3.1.0, 3.2.0 Feb 27, 2024
@zacharyburnett zacharyburnett self-requested a review May 7, 2024 17:56
@braingram
Copy link
Contributor Author

Thanks again! I made a minor addition to the comment before the pass to include some details from the discussion here. It will hopefully serve as a reminder for future me (or whomever) encounters the pass in the future.

@braingram braingram merged commit bf9dbf3 into asdf-format:main May 7, 2024
48 checks passed
@braingram braingram deleted the asdf_traverse branch May 7, 2024 20:25
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.

Objects that implement __asdf_traverse__ are expected to have a _tag
2 participants