-
Notifications
You must be signed in to change notification settings - Fork 58
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
Allow converter deferral and move Reference to a converter #1561
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
braingram
force-pushed
the
select_tag_none
branch
from
June 9, 2023 21:35
031d46a
to
8747c85
Compare
braingram
requested review from
eslavich and
perrygreenfield
and removed request for
a team
June 12, 2023 15:26
py38-devdeps failures are expected and addressed in: #1556 |
braingram
force-pushed
the
select_tag_none
branch
from
June 16, 2023 20:52
f85600b
to
567bc44
Compare
braingram
force-pushed
the
select_tag_none
branch
2 times, most recently
from
June 27, 2023 17:07
b19ce48
to
ae81c6e
Compare
WilliamJamieson
approved these changes
Jul 3, 2023
by Converter.select_tag returning None asdf can use Converter.to_yaml_tree to convert to a new type which asdf will then use to search for a new converter (used converters are tracked to avoid conversion cycles)
braingram
force-pushed
the
select_tag_none
branch
from
July 12, 2023 14:06
ae81c6e
to
8601294
Compare
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Jul 18, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Jul 18, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Jul 31, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Aug 2, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
Merged
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Aug 16, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Aug 18, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Aug 18, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Aug 18, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Aug 18, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Sep 8, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Sep 11, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
braingram
added a commit
to braingram/asdf
that referenced
this pull request
Sep 11, 2023
Prior to asdf-format#1561 the types supported by a converter that doesn't support any tags that are supported by the extnesion were ignored. The modifications in that PR checked if the converter implemented 'select_tag' and if not ignored the types. However if the converter implemented 'select_tag' the converter types were indexed (as the converter could return None from 'select_tag' and defer to a new converter). If a converter inherits from Converter it gains the select_tag method from the parent Converter class (which provides documentation of the method but doesn't implement functionality that is otherwise covered by ConverterProxy). This creates an issue when indexing the converter tags/types if a converter is included in an extension that doesn't list support for the tags supported by the converter. The changes included here check if 'select_tag' is implemented by a subclass of Converter. If overwritten, the types of the converter are indexed (to allow the converter to defer conversion). However, if 'select_tag' is merely inherited by Converter, the types supported by the converter are ignored (agreeing with behavior prior to asdf-format#1561).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following the ASDF tag-up discussion here is another option for moving Reference to a converter based on the suggestions from @eslavich
asdf.reference.Reference
serializes to an untagged dictionary.asdf/asdf/reference.py
Lines 45 to 46 in b0c9a50
asdf/asdf/reference.py
Lines 108 to 111 in b0c9a50
This type of conversion is unsupported in the new extension API (using Converters) in asdf main. To allow
Reference
to be moved to aConverter
this PR introduces the ability to define converters that defer conversion to other converters (or in the case ofReference
to un-tagged serialization to a yaml map).This PR changes the public API of
Converter.select_tag
(in a backwards compatible way) allowing it to returnNone
when asdf should convert the object being serialized with a different converter. Whenselect_tag
returns None, the deferringConverter.to_yaml_tree
is called on the object being serialized to convert the object instance from one type (the one that triggered deferral) to another type (which can either then defer to another converter or return a valid tag and serialize the object).In the case of
Reference
the newReferenceConverter.select_tag
returns None andReferenceConverter.to_yaml_tree
returns a 'vanilla' dictionary which will be converted to an untagged yaml map.This converter deferral can be used to support subclasses that can be serialized as the superclass (which was done automatically with the old extension API) and docs were updated to describe options for how subclasses can be supported. The changes in this PR provide a migration path for user code that previously relied on ndarray subclass conversion as described in #1537