Rename the misspelled BuildOptions check_for_cyces keyword - #167
Merged
Merged
Conversation
The BuildOptions constructor advertised check_for_cyces while assigning the correctly spelled check_for_cycles attribute, so the signature disagreed with both the attribute docstrings and docs/library.rst. Callers passing the documented check_for_cycles=False only worked by accident: the **kwargs catch-all set the unrecognized name as an attribute, and it ran after the misspelled parameter had already been assigned, so it happened to overwrite the same attribute. Rename the parameter to check_for_cycles and keep check_for_cyces as a deprecated alias. The alias is popped out of kwargs before the catch-all runs, so it no longer leaves a misspelled junk attribute behind, and it raises a DeprecationWarning that points callers at the new spelling. An out-of-tree caller that passes both spellings now gets a TypeError rather than a silently chosen value, because BuildOptions.__getattr__ returns False for anything unset and would otherwise hide the mistake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GypKU5KdLfs2Cf8kS2TzJa
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.
Closes #157
BuildOptions.__init__advertised a keyword namedcheck_for_cycesbut assigned it to the correctly spelledcheck_for_cyclesattribute, so the signature disagreed with the attribute docstrings, the API docs, anddocs/library.rst. Callers passing the documentedcheck_for_cycles=Falseworked only by accident: the**kwargscatch-all set the unrecognized name as an attribute, and that loop ran after the misspelled parameter had already been assigned, so it happened to overwrite the same attribute.This renames the parameter to
check_for_cyclesand keepscheck_for_cycesas a deprecated alias:kwargsbefore the catch-all runs, so it no longer leaves a misspelled junk attribute behind.DeprecationWarningnaming the new spelling.TypeErrorinstead of silently choosing one. Without this, a mistake would be invisible, becauseBuildOptions.__getattr__returnsFalsefor any attribute that was never set.Nothing in this repository passes either spelling, so the alias exists only for out-of-tree callers. It can be removed in the next release that allows breaking changes, such as 0.4.0.
The
**kwargscatch-all is left as is. Rejecting unknown keywords would be a separate breaking change beyond the scope of this issue.Validation
The four new tests in
test/test_builder.pywere run against the unfixed code first: all four failed (noDeprecationWarning, noTypeError, andcheck_for_cyclesmissing from the constructor signature whilecheck_for_cyceswas present). They pass after the fix.uv run --frozen --extra dev ruff check graphtage test docs bindist: passesuv run --frozen --extra dev pytest: 144 passeduv run --frozen --extra dev make -C docs html SPHINXOPTS="-W --keep-going": build succeededuv lock --check: reports the lockfile needs updating, but that reproduces on an unmodifiedmastercheckout in this environment because of a globalexclude-newersetting. Neitherpyproject.tomlnoruv.lockis touched by this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01GypKU5KdLfs2Cf8kS2TzJa