FIX: honor include_subclasses=False for the registered class itself - #2471
Merged
Roman Lutz (romanlutz) merged 3 commits intoAug 24, 2026
Merged
Conversation
Roman Lutz (romanlutz)
requested changes
Aug 24, 2026
…itself GlobalDefaultValues.get_default_value() previously only looked up the exact scope with include_subclasses=True. A default registered with include_subclasses=False was therefore unreachable for every class - including the very class it was registered for - because the exact-match probe used the wrong flag and the parent-class fallback loop skips registrations whose flag is False. The flag's documented meaning is 'apply to subclasses as well', so a False registration must still apply to the registered class itself while stopping short of inheritance. The lookup now probes both flags for the exact (class_type, parameter_name) scope before falling back to the inheritance loop. Added regression tests: no-subclass default resolves for the base class but not the child, and mixed-flag registrations on base/child resolve independently.
fei (feiiiiii5)
force-pushed
the
fix/default-subclass-flag
branch
from
August 24, 2026 06:15
5279cfa to
66a1e17
Compare
…s the old scope set_default_value() now removes the opposite include_subclasses scope for the same (class, parameter) before storing, so True-then-False re-registration lets the newer value win for the registered class and stops subclasses from inheriting the removed True-scope default (lookup checks the True scope first). Regression tests cover both flag directions. Signed-off-by: fei <204683769+feiiiiii5@users.noreply.github.com>
Roman Lutz (romanlutz)
approved these changes
Aug 24, 2026
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
2 participants
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.
Description
GlobalDefaultValues.get_default_value()only probed the exact scope withinclude_subclasses=True. A default registered withinclude_subclasses=Falsewas therefore unreachable for every class — including the class it was registered for — because the exact-match probe used the wrong flag and the parent-class fallback loop skips registrations whose flag isFalse.The flag is documented as "whether this default should apply to subclasses as well", so a
Falseregistration must still apply to the registered class itself while stopping short of inheritance.Change
The exact-match lookup now tries both registration flags on the
(class_type, parameter_name)scope before falling back to the inheritance loop. This also means aFalseregistration no longer needs the fallback loop at all for its own class.Tests
test_global_default_values_no_subclass_still_applies_to_base—Falseregistration resolves for the base class, not the child (pre-fix: FAIL — value was unreachable).test_global_default_values_mixed_flags_same_param— base-onlyFalse+ childTrueregistrations on the same parameter resolve independently (pre-fix: FAIL on the child-visible case).tests/unit/common/test_apply_defaults.py+test_pyrit_default_value.py→ 66 passed; ruff check + format clean.git checkout HEAD~1 -- <file>: both new tests fail on the old implementation.