Use ChainMap to improve representer/constructor inheritance
#672
Conversation
Before this change, a child Dumper "inherited" its parent Dumper's representers by simply copying the parent's representers the first time a new representer was registered with the child Dumper. This approach works as users expect only if representers are never added to a parent (ancestor) Dumper after representers are added to a child (descendant) Dumper. Same goes for Loaders and their registered constructors. This commit uses a `collections.ChainMap` built from ancestor `dict` objects in method resolution order (MRO) to provide true inheritance of representers (for Dumpers) and constructors (for Loaders). This is technically a backwards-incompatible change: This change breaks any code that intentionally subverts the expected inheritance behavior by registering a function in an ancestor class *after* registering a function in a descendant class.
|
Beyond the case you bring up (which is admittedly pretty obscure), there's a bigger problem with this approach at a glance: it makes it effectively impossible for objects later in the MRO to explicitly remove (not replace/re-define) inherited entries, which is actually more common than one might think. That case could probably be handled by convention with a surrogate sentinel value or something, but it kinda doesn't feel right... The prototype in #700 addresses this problem in a different way, which is probably a little more aligned with where things are going with type/tag/schema support for 1.2. Each runtime type ultimately still has its own flat list, but they're explicitly configured from shared first-class definitions that are usage-specific (and support inheritance themselves). I'm going to close this one- sorry it's taken so long for us to look at it... |
Before this change, a child Dumper "inherited" its parent Dumper's representers by simply copying the parent's representers the first time a new representer was registered with the child Dumper. This approach works as users expect only if representers are never added to a parent (ancestor) Dumper after representers are added to a child (descendant) Dumper. Same goes for Loaders and their registered constructors.
This commit uses a
collections.ChainMapbuilt from ancestordictobjects in method resolution order (MRO) to provide true inheritance of representers (for Dumpers) and constructors (for Loaders).This is technically a backwards-incompatible change: This change breaks any code that intentionally subverts the expected inheritance behavior by registering a function in an ancestor class after registering a function in a descendant class.