Repository navigation
Conversation
Setting a class default on a subclass copies the inherited Parameter into it, but the cached class parameters of the subclass (and of its own subclasses) still pointed to the superclass Parameter, so instantiate=True parameters were deep-copied from the old default. Clear those caches when the copy is made.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1219 +/- ##
==========================================
+ Coverage 89.25% 89.49% +0.23%
==========================================
Files 9 9
Lines 6517 6674 +157
==========================================
+ Hits 5817 5973 +156
- Misses 700 701 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # The cached class parameters of mcs and its subclasses may | ||
| # still point to the superclass Parameter that was just copied. | ||
| for cls in descendents(t.cast("type[t.Any]", mcs)): | ||
| cls._param__private.params.clear() |
There was a problem hiding this comment.
Do we need to clear the whole cache or can we just clear the cache entry for the overridden parameter?
There was a problem hiding this comment.
Good point, we don't need to. Each subclass's cache only holds a stale entry for this one name, and only where it still resolves to the inherited Parameter. So now only that entry is swapped for the new copy, and params_to_deepcopy / params_to_ref / params_with_default_factory are reset to None, which _cls_parameters already rebuilds on the next lookup. Subclasses that define their own p keep their entry untouched, and I added that case to the test.
Fixes #1218
Setting a class-level default on a subclass, e.g.
Looked.d = {'x': 1}, copies the inheritedParameterinto the subclass inParameterizedMetaclass.__setattr__. IfLooked.param[...]had been called before,_cls_parameterswas already cached. That cache, includingparams_to_deepcopy, still held the superclassParameter. So forinstantiate=Trueparameters, new instances deep-copied the old default.When the copy is made, this clears the cached class parameters of the class and its subclasses (through
descendents), so they are rebuilt on the next lookup. A subclass that had also been looked up would otherwise keep the staleParametertoo.add_parameterclears the cache the same way.The new test covers the issue example plus a cached sub-subclass, and fails without the change. The full test suite passes locally (1787 passed).