Skip to content

Commit ecf1547

Browse files
committed
Fix method resolution order of defaults for to_tap_class
1 parent 4eea684 commit ecf1547

File tree

3 files changed

+133
-8
lines changed

3 files changed

+133
-8
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,7 @@ special argument behavior. For example, you can override [`configure`](#configur
874874
[`process_args`](#argument-processing).
875875

876876
If the object can be `tapify`d, then it can be `to_tap_class`d, and vice-versa. `to_tap_class` provides full control
877-
over argument parsing.
877+
over argument parsing. Just as in `tapify`, the data extracted from standard Python classes (not `pydantic` models or `dataclass`es) only comes from the `__init__`, not from class variables.
878878

879879
### `to_tap_class` examples
880880

src/tap/tapify.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,14 @@ class ArgParser(Tap):
281281
def _configure(self):
282282
for arg_data in args_data:
283283
variable = arg_data.name
284-
self._annotations[variable] = str if arg_data.annotation is Any else arg_data.annotation
285-
self.class_variables[variable] = {"comment": arg_data.description or ""}
286-
if arg_data.is_required:
287-
kwargs = {}
288-
else:
289-
kwargs = dict(required=False, default=arg_data.default)
290-
self.add_argument(f"--{variable}", **kwargs)
284+
if variable not in self.class_variables:
285+
self._annotations[variable] = str if arg_data.annotation is Any else arg_data.annotation
286+
self.class_variables[variable] = {"comment": arg_data.description or ""}
287+
if arg_data.is_required:
288+
kwargs = {}
289+
else:
290+
kwargs = dict(required=False, default = arg_data.default)
291+
self.add_argument(f"--{variable}", **kwargs)
291292

292293
super()._configure()
293294

tests/test_to_tap_class.py

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,3 +547,127 @@ def test_subclasser_subparser_help_message(
547547
_test_subclasser_message(
548548
subclasser_subparser, class_or_function_, expected_message, description=description, args_string=args_string
549549
)
550+
551+
class TestMethodResolutionOrder:
552+
@staticmethod
553+
def _assert_all_two(class_or_function: Any) -> None:
554+
class ParserClass(to_tap_class(class_or_function)):
555+
a: int = 2
556+
557+
assert ParserClass.a == ParserClass().a == ParserClass().parse_args([]).a == 2
558+
559+
def test_function(self):
560+
def foo(a: int = 0):
561+
...
562+
563+
self._assert_all_two(foo)
564+
565+
def test_plain_class(self):
566+
class Foo:
567+
def __init__(self, a: int = 0):
568+
self.a = a
569+
570+
self._assert_all_two(Foo)
571+
572+
def test_pydantic_model(self):
573+
class Foo(pydantic.BaseModel):
574+
a: int = 0
575+
576+
self._assert_all_two(Foo)
577+
578+
def test_dataclass(self):
579+
@dataclasses.dataclass
580+
class Foo:
581+
a: int = 0
582+
583+
self._assert_all_two(Foo)
584+
585+
def test_configure_in_leaf(self):
586+
def foo(a: int = 0):
587+
...
588+
589+
class Leaf(to_tap_class(foo)):
590+
a: int = 2
591+
592+
def configure(self) -> None:
593+
# Add an extra argument to validate that leaf.configure runs after _configure in foo
594+
self.add_argument("--a", default=3)
595+
596+
assert Leaf.a == Leaf().a == 2
597+
assert Leaf().parse_args([]).a == 3
598+
599+
def test_class_to_tap_class_thats_subclassed(self):
600+
class Parent:
601+
def __init__(self, a: int = 0):
602+
self.a = a
603+
604+
class TapChild(to_tap_class(Parent)):
605+
a: int = 1
606+
607+
class TapGrandchild(TapChild):
608+
a: int = 2
609+
610+
assert TapChild.a == TapChild().a == TapChild().parse_args([]).a == 1
611+
assert TapGrandchild.a == TapGrandchild().a == TapGrandchild().parse_args([]).a == 2
612+
613+
def test_class_subclass_to_tap_class(self):
614+
class Parent:
615+
def __init__(self, a: int = 0):
616+
self.a = a
617+
618+
class TapChild(Parent):
619+
a: int = 1
620+
621+
class TapGrandchild(to_tap_class(TapChild)):
622+
a: int = 2
623+
624+
assert TapGrandchild.a == TapGrandchild().a == TapGrandchild().parse_args([]).a == 2
625+
626+
def test_inheritance(self):
627+
class Parent:
628+
def __init__(self, a: int = 0, b: int = 5):
629+
self.a = a
630+
self.b = b
631+
632+
class TapChild(Parent):
633+
def __init__(self, a: int = 0, b: int = 5, c: int = 3):
634+
super().__init__(a=a, b=b)
635+
self.c = c
636+
637+
class TapGrandchild(to_tap_class(TapChild)):
638+
a: int = 2
639+
640+
assert TapGrandchild.a == TapGrandchild().a == TapGrandchild().parse_args([]).a == 2
641+
assert TapGrandchild().parse_args([]).b == 5
642+
assert TapGrandchild().parse_args([]).c == 3
643+
644+
def test_defaults_and_required(self):
645+
class Parent:
646+
def __init__(self, a: int, e: int, b: int = 5):
647+
self.a = a
648+
self.b = b
649+
self.e = e
650+
651+
class TapChild(Parent):
652+
def __init__(self, b: int, a: int = 0, c: int = 3):
653+
super().__init__(a=a, b=b, e=6)
654+
self.c = c
655+
656+
class TapGrandchild(to_tap_class(TapChild)):
657+
a: int = 2
658+
d: int
659+
660+
args = ["--b", "6", "--c", "5", "--d", "4"]
661+
assert TapGrandchild.a == TapGrandchild().a == TapGrandchild().parse_args(args).a == 2
662+
assert TapGrandchild().parse_args(args).b == 6
663+
assert TapGrandchild().parse_args(args).c == 5
664+
assert TapGrandchild().parse_args(args).d == 4
665+
with pytest.raises(AttributeError):
666+
TapGrandchild().parse_args(args).e
667+
668+
args = ["--b", "6"]
669+
with pytest.raises(SystemExit):
670+
TapGrandchild().parse_args(args)
671+
args = ["--d", "4"]
672+
with pytest.raises(SystemExit):
673+
TapGrandchild().parse_args(args)

0 commit comments

Comments
 (0)