diff --git a/docs/approach_vs_tool.md b/docs/approach_vs_tool.md index d8b1200..6522204 100644 --- a/docs/approach_vs_tool.md +++ b/docs/approach_vs_tool.md @@ -4,13 +4,27 @@ As this module's code generation is inspired by the workings of [David Beazley's I thought it was briefly worth discussing his note on learning an approach vs using a tool. I think that learning an approach is valuable, this module would not exist without the -example given by `cluegen`. It also wouldn't exist if I hadn't needed to extend `cluegen` -for some basic features (try using `Path` default values with `cluegen`). +example given by `cluegen`. + +However, what I foundn was that in essentially every case where I wanted to use +these generating tools, I needed to modify them - often significantly. +It quickly became easier to just create my own tool and upload it as a package. + +For example, `cluegen` has a few subtle "exercises for the reader". It needs extending +and fixing for some use-cases. + * Default values that are not builtins need to be passed as part of the globals + dict to `exec`. + * No support for mutable defaults. + * Subclass methods will be overwritten if they call a cluegen method that has not been + generated via `super().methodname(...)` + * `inspect.signature(cls)` does not work if `cls.__init__` has not already been generated. + (I think this is actually a bug in inspect). + * Need an extra filter to support things like `ClassVar`. In the general spirit though, this module intends to provide some basic tools to help -build your own custom class generators. +create your own customized boilerplate generators. The generator included in the base module is intended to be used to help 'bootstrap' a modified generator with features that work how **you** want them to work. -The `prefab` module is the more fully featured powertool *I* built with these tools. -However, much like a prefabricated building, it may not be what you desire. +The `prefab` module is the more fully featured tool that handles the additional cases *I* +needed. diff --git a/src/ducktools/classbuilder/__init__.py b/src/ducktools/classbuilder/__init__.py index 285ea21..0177068 100644 --- a/src/ducktools/classbuilder/__init__.py +++ b/src/ducktools/classbuilder/__init__.py @@ -160,19 +160,39 @@ def __repr__(self): def __get__(self, inst, cls): local_vars = {} - gen = self.code_generator(cls, self.funcname) + + # This can be called via super().funcname(...) in which case the class + # may not be the correct one. If this is the correct class + # it should have this descriptor in the class dict under + # the correct funcname. + # Otherwise is should be found in the MRO of the class. + if cls.__dict__.get(self.funcname) is self: + gen_cls = cls + else: + for c in cls.__mro__[1:]: # skip 'cls' as special cased + if c.__dict__.get(self.funcname) is self: + gen_cls = c + break + else: # pragma: no cover + # This should only be reached if called with incorrect arguments + # manually + raise AttributeError( + f"Could not find {self!r} in class {cls.__name__!r} MRO." + ) + + gen = self.code_generator(gen_cls, self.funcname) exec(gen.source_code, gen.globs, local_vars) method = local_vars.get(self.funcname) try: - method.__qualname__ = f"{cls.__qualname__}.{self.funcname}" + method.__qualname__ = f"{gen_cls.__qualname__}.{self.funcname}" except AttributeError: # This might be a property or some other special # descriptor. Don't try to rename. pass # Replace this descriptor on the class with the generated function - setattr(cls, self.funcname, method) + setattr(gen_cls, self.funcname, method) # Use 'get' to return the generated function as a bound method # instead of as a regular function for first usage. diff --git a/tests/test_core.py b/tests/test_core.py index dc6a9de..46a1a63 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -8,6 +8,7 @@ builder, default_methods, + eq_maker, get_fields, get_flags, get_methods, @@ -527,3 +528,33 @@ class SigClass: __slots__ = SlotFields(x=42) assert str(inspect.signature(SigClass)) == "(x=42)" + + +def test_subclass_method_not_overwritten(): + @slotclass + class X: + __slots__ = SlotFields(x=Field()) + + class Y(X): + def __init__(self, x, y): + self.y = y + super().__init__(x=x) + + y_init_func = Y.__init__ + + assert X.__dict__["__eq__"] is eq_maker + + y_inst = Y(0, 1) + + # super().__init__ method generated correctly + assert y_init_func is Y.__init__ + assert X.__dict__["__init__"] is not init_maker + assert (y_inst.x, y_inst.y) == (0, 1) + + # Would fail previously as __init__ would be overwritten + y_inst_2 = Y(0, 2) + + assert y_inst == y_inst_2 + + assert X.__dict__["__eq__"] is not eq_maker + assert "__eq__" not in Y.__dict__