Add warning when conversion fails with default warn_on_failed_conversion - #2125
Conversation
braingram
left a comment
There was a problem hiding this comment.
Thanks for putting this together. I left a few suggestions.
What's your take/plan for the downstream errors?
| warnings.warn(f"A node failed to convert with: {err}", AsdfConversionWarning) | ||
| obj = node | ||
| else: | ||
| if not is_set(get_config()).warn_on_failed_conversion: |
There was a problem hiding this comment.
What are options for consolidating this code with the updates in lazy_nodes to avoid duplicating the error message?
There was a problem hiding this comment.
Idk the message is only in two places and will be removed following the next major release, so to me it doesn't seem like a problem. If we really wanted to avoid duplication we could create an AsdfFutureWarning subtype with that text as its message, but that seems like more overhead than the current approach.
|
@braingram the downstream errors are pretty simple to fix, generally they just require manually setting |
braingram
left a comment
There was a problem hiding this comment.
Thanks! This warning will hopefully inform some downstream users and the docs and details helpfully describe the upcoming changes and how code can be adapted.
Description
Part of #2127
is_setfunction which can be used to check if attributes of an object have been modified along with an_IsSetbase class necessary for it to functionasdf.exceptions.AsdfFutureWarningwarn_on_failed_conversionhasn't been manually set thenAsdfFutureWarningis emitted before an exception is raised.PRs to address downstream test failures:
AI Disclosure
No AI tools used
Tasks
prekon your machinepyteston your machineno-changelog-entry-needed)changes/:echo "changed something" > changes/<PR#>.<changetype>.rst(see below for change types)docs/pagenews fragment change types...
changes/<PR#>.feature.rst: new featurechanges/<PR#>.bugfix.rst: bug fixchanges/<PR#>.doc.rst: documentation changechanges/<PR#>.removal.rst: deprecation or removal of public APIchanges/<PR#>.general.rst: infrastructure or miscellaneous change