register_type replaces instead of failing, applies to subscripted types, and warns for callable defaults that can't be imported back - #966
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #966 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 28 28
Lines 9235 9250 +15
=========================================
+ Hits 9235 9250 +15 ☔ View full report in Codecov by Harness. |
|
This file contains hidden or 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
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.



What does this PR do?
Replacing a registration.
register_typefailed when the type was already registered with a different serializer/deserializer. That made every new type registered by jsonargparse a breaking change for code that already registers it, e.g. a package registeringos.PathLikebefore jsonargparse did. The new registration now replaces the previous one, and a debug log names the module of each, which is what you need to see when two packages register the same type.fail_already_registered=Truekeeps the old behavior. This also removes the_fail_already_registeredglobal that jsonargparse used internally to exempt its own registrations from the check.Subscripted types. A registered generic was ignored as soon as it was subscripted:
os.PathLikeworked butos.PathLike[str]did not.get_registered_typenow falls back to the registration of the origin, so both spellings behave the same. Types are registered unsubscripted and the type arguments are not validated, since the deserializer gets the complete value.Callable defaults. A
Callabledefault whose import path contains<locals>, e.g. a closure, was dumped as that path, which looks valid but can't be imported back. The default is now kept as the object, sodumpgives the usualUnable to serialize instance ...message together with a warning, instead of silently producing an unusable config.Also:
serialize_class_instanceand the serialization branches for callables and class instances now go through a singleserialize_as_import_pathbuilt onobject_path_serializer, which is where the importability check already lived, andUntypedType.__init__delegates to its parent instead of setting the attributes itself.Before submitting