From 6241667f22cec6469acdc205fd38885486e7a397 Mon Sep 17 00:00:00 2001 From: Evan Sultanik Date: Wed, 9 Sep 2026 10:19:10 -0400 Subject: [PATCH] Rename the misspelled BuildOptions check_for_cyces keyword 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 Claude-Session: https://claude.ai/code/session_01GypKU5KdLfs2Cf8kS2TzJa --- graphtage/graphtage.py | 48 ++++++++++++++++++++++++++++++++++++++++-- test/test_builder.py | 28 ++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/graphtage/graphtage.py b/graphtage/graphtage.py index 5d883ee..14f3496 100644 --- a/graphtage/graphtage.py +++ b/graphtage/graphtage.py @@ -2,6 +2,7 @@ import copy as copy_module import mimetypes +import warnings from abc import ABC, ABCMeta, abstractmethod from collections.abc import Collection, Iterable, Iterator from typing import Any, Generic, TypeVar @@ -1014,7 +1015,7 @@ def __init__(self, *, allow_list_edits=True, allow_list_edits_when_same_length=True, ignore_list_order=False, - check_for_cyces=True, + check_for_cycles: bool | None = None, ignore_cycles=False, printer=NULL_PRINTER, **kwargs @@ -1023,7 +1024,16 @@ def __init__(self, *, Options not specified will default to :const:`False`. + Args: + check_for_cycles: Whether to check the input for cycles. Omitting it, or passing :const:`None`, + selects the default of :const:`True`. The misspelled ``check_for_cyces`` is still accepted as a + deprecated alias and raises a :class:`DeprecationWarning`. + + Raises: + TypeError: If both ``check_for_cycles`` and the deprecated ``check_for_cyces`` alias are given. + """ + check_for_cycles = self._resolve_check_for_cycles(check_for_cycles, kwargs) self.allow_key_edits = allow_key_edits """Whether to consider editing keys when matching :class:`KeyValuePairNode` objects""" self.allow_list_edits = allow_list_edits @@ -1048,7 +1058,7 @@ def __init__(self, *, """ self.auto_match_keys = auto_match_keys """Whether to automatically match key/value pairs in dictionaries if they share the same key""" - self.check_for_cycles = check_for_cyces + self.check_for_cycles = check_for_cycles """If possible, check for cycles in the input If `True` and if `ignore_cycles` is `False`, then a :class:`ValueError` will be raised if a cycle is detected @@ -1063,6 +1073,40 @@ def __init__(self, *, for attr, value in kwargs.items(): setattr(self, attr, value) + @staticmethod + def _resolve_check_for_cycles(check_for_cycles: bool | None, kwargs: dict[str, Any]) -> bool: + """Resolves the ``check_for_cycles`` option, honoring its deprecated misspelling. + + The alias is removed from ``kwargs`` so that it is not also set as an attribute by the catch-all that + assigns every unrecognized keyword. + + Args: + check_for_cycles: The value given for the correctly spelled keyword, or :const:`None` if it was omitted. + kwargs: The unrecognized keyword arguments, modified in place. + + Returns: + bool: The value to assign to :attr:`BuildOptions.check_for_cycles`. + + Raises: + TypeError: If both spellings were given. + + """ + deprecated = kwargs.pop("check_for_cyces", None) + if deprecated is not None: + warnings.warn( + "The `check_for_cyces` keyword argument of BuildOptions is misspelled and deprecated; " + "use `check_for_cycles` instead.", + DeprecationWarning, + stacklevel=3, + ) + if check_for_cycles is not None: + raise TypeError( + "BuildOptions got both `check_for_cycles` and its deprecated alias `check_for_cyces`; " + "pass only `check_for_cycles`" + ) + check_for_cycles = deprecated + return True if check_for_cycles is None else check_for_cycles + def copy(self) -> "BuildOptions": return copy_module.copy(self) diff --git a/test/test_builder.py b/test/test_builder.py index f1d93eb..7c8f6e9 100644 --- a/test/test_builder.py +++ b/test/test_builder.py @@ -1,3 +1,4 @@ +from inspect import signature from unittest import TestCase from graphtage import BuildOptions, IntegerNode, ListNode, TreeNode, UnorderedListNode @@ -37,3 +38,30 @@ def build_foo(self, obj: Foo, children: list[TreeNode]): tree = Tester().build_tree(Foo(10)) self.assertIsInstance(tree, IntegerNode) self.assertEqual(10, tree.object) + + +class TestBuildOptions(TestCase): + def test_check_for_cycles(self): + parameters = signature(BuildOptions.__init__).parameters + self.assertIn("check_for_cycles", parameters) + self.assertNotIn("check_for_cyces", parameters) + self.assertTrue(BuildOptions().check_for_cycles) + self.assertFalse(BuildOptions(check_for_cycles=False).check_for_cycles) + + def test_deprecated_check_for_cycles_spelling(self): + with self.assertWarns(DeprecationWarning): + options = BuildOptions(check_for_cyces=False) + self.assertFalse(options.check_for_cycles) + with self.assertWarns(DeprecationWarning): + options = BuildOptions(check_for_cyces=True) + self.assertTrue(options.check_for_cycles) + + def test_check_for_cycles_sets_no_misspelled_attribute(self): + self.assertNotIn("check_for_cyces", vars(BuildOptions(check_for_cycles=False))) + with self.assertWarns(DeprecationWarning): + options = BuildOptions(check_for_cyces=False) + self.assertNotIn("check_for_cyces", vars(options)) + + def test_conflicting_check_for_cycles_spellings(self): + with self.assertRaises(TypeError): + BuildOptions(check_for_cycles=True, check_for_cyces=False)