fix: lazyproperty re-evaluates getter when cached value is None - #1602
Open
afonsojanu wants to merge 1 commit into
Open
fix: lazyproperty re-evaluates getter when cached value is None#1602afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
lazyproperty.__get__ checked whether the cached value was None to decide if the getter had run yet, so any getter that legitimately returns None got re-evaluated on every access instead of being cached after the first call. Check for key presence in the instance __dict__ instead, which distinguishes "not yet computed" from "computed None" correctly.
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
1 participant
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.
Fixes #1600.
lazyproperty.__get__decided whether the getter had already run by checkingif value is None, using the instance__dict__lookup's ownNonedefault as the "not computed yet" signal. That works fine as long as the getter never legitimately returnsNone, but if it does, the cachedNonelooks identical to an unset value and the getter runs again on every single access, silently breaking the "evaluated only on first access" guarantee this class documents and exists to provide.The fix checks for key presence in
obj.__dict__instead of comparing the value toNone, so a getter that returnsNoneis cached correctly after its first call, same as any other value.Added two tests in
tests/test_shared.py::DescribeLazypropertycovering both a getter that returnsNoneand one that doesn't, asserting the getter's call count stays at 1 after multiple accesses in each case. Confirmed the newNone-returning test fails against the old implementation (3 calls instead of 1) and passes against the fix.Ran the full local suite (
uv run pytest -x, 1611 passed),ruff check/ruff format --checkon the changed files, andpyright(clean on the changed files; the one pyright error surfaced is inside a bundled typeshed stub, unrelated to this change and pre-existing on master).