2

I have a parent class that is inherited by several children. I would like to initialize one of the children using the parent's @classmethod initializers. How can I do this? I tried:

class Point(object):
    def __init__(self,x,y):
        self.x = x
        self.y = y

    @classmethod
    def from_mag_angle(cls,mag,angle):
        x = mag*cos(angle)
        y = mag*sin(angle)
        return cls(x=x,y=y)


class PointOnUnitCircle(Point):
    def __init__(self,angle):
        Point.from_mag_angle(mag=1,angle=angle)


p1 = Point(1,2)
p2 = Point.from_mag_angle(2,pi/2)
p3 = PointOnUnitCircle(pi/4)
p3.x #fail
alex
  • 2,968
  • 3
  • 23
  • 25
  • 2
    `@classmethod` and `self`?! Those don't really go together. – deceze Nov 10 '15 at 14:25
  • This whole concoction looks pretty suspicious and would not qualify as good OO code in any language, I'd say. – deceze Nov 10 '15 at 14:27
  • 1
    The first parameter of a class method is usually named `cls`, to distinguish it from the instance `self`. You can certainly access class methods via `self`, but in this instance it's not clear why you'd want to, as the class method sets a class attribute. – jonrsharpe Nov 10 '15 at 14:28
  • sorry for the errors, i fixed them. – alex Nov 10 '15 at 14:49
  • 2
    You can't assign to `self` in `__init__` and expect that to work. If the class methods are alternate constructors, why not just use `A10.from_half_a(5)`? – jonrsharpe Nov 10 '15 at 14:50
  • What are you trying to achieve? Have you read about the factory design pattern? – Raydel Miranda Nov 10 '15 at 14:50
  • 1. A `@classmethod` is not an initializer. [Edited to remove points that were already fixed while I was typing this.] 4. Assigning to `self` in an initializer won't achieve anything, especially if it's the last line of `__init__`. ... What on Earth are you _trying_ to do? Specifically, what do you want an initialized child instance to look like, and why won't any of the usual ways of doing that work for you? – Kevin J. Chase Nov 10 '15 at 14:53
  • @KevinJ.Chase, i changed the syntax to be correct which should answer some of your questions. basically, i would for A10 (and others like it) to be initialized using its parent's classmethod. there are other arguments for each class's __init__ which i omitted for simplicity, but perhaps it is too misleading. also i use `@classmethods` as alternative constructors all the time. – alex Nov 10 '15 at 14:55
  • I've removed points 2 and 3, which you have already corrected. I second @jonrsharpe though --- the whole point of the `cls` parameter in a `@classmethod` is so you can return an instance of a subclass, so there's no need to invoke it in a subclass' `__init__` method. It already does what you want. – Kevin J. Chase Nov 10 '15 at 14:59
  • 1
    The class method calls `__init__`, not the other way around. Inheritance already does what you want, because you've used `cls(...)` rather than `A(...)` in the inherited method. – jonrsharpe Nov 10 '15 at 15:00
  • i suppose i disagree. frequently classes can be initialized in many ways. i use @classmethod's like `from_this` and `from_that` to create alternative constructors. so what i am trying to do is simply use a parent's alternative constructor from a child class. – alex Nov 10 '15 at 15:02
  • 3
    @alex this isn't a matter of opinion! When you call `cls(...)` from inside the class method, that invokes `__init__` (and/or `__new__`) on whatever class `cls` represents, creating a new instance. *"what i am trying to do is simply use an parent's alternative constructor from a child class"*, so, **again**, just use `ChildClass.inherited_class_method(...)`. See e.g. http://stackoverflow.com/q/1015592/3001761, http://stackoverflow.com/q/1216356/3001761 for why you can't assign to `self`. – jonrsharpe Nov 10 '15 at 15:04
  • 1
    A constructor is not an initializer. If it were, it would have a reference to an already-constructed instance. Since `from_half_a` has exactly two parameters --- `cls` and `half_a` --- and _neither_ is a reference to some newly-constructed instance, it _can't_ be an initializer. – Kevin J. Chase Nov 10 '15 at 15:05
  • so perhaps my usage of classmethods to implement alternative initialization forms is bad practice? whats the preferred method? – alex Nov 10 '15 at 15:14
  • A class method is a good way to implement alternate *constructors*. Why not just use instance methods, if you want to initialise instance attributes? Or you might need to look into using `__new__`. Could you provide a less abstract example of what you're actually trying to *achieve*? Note you need to include `@username` when responding to other comments. – jonrsharpe Nov 10 '15 at 15:21
  • That alternative constructor is fine, just your expectation of its usage seems weird. `A10()` should initialise the class normally, `A10.from_half_a()` would be its alternative constructor. Exactly the same as for its parent class! Why should `A10()` suddenly use `A`'s alternative constructor? – deceze Nov 10 '15 at 15:23
  • 1
    "Alternative initializer"? If you're already creating subclasses, you've already got your alternative initializer --- it's called `SubclassOfA.__init__`. That doesn't seem to be the kind of answer you want, but at this point I have _no idea_ what you want. You've hinted that there's something much more complex going on, but not what it is, nor why the typical uses of `__init__` or `@classmethod` won't work for you. – Kevin J. Chase Nov 10 '15 at 15:26
  • 1
    Normally one doesn't explain upvotes on this site, but I wanted to highlight how drastically your edit that added a _specific_ use case changed everything. This started as a long, frustrating argument in the comments about terminology and intent, in part because the abstract `class A` example was too simple to demonstrate your problem. Once you showed us what you were _really_ trying to do, this became a surprisingly good question, which got a useful (to you and probably to others) answer from @jonrsharpe less than 10 minutes later. – Kevin J. Chase Nov 10 '15 at 22:40

2 Answers2

3

If you try to write __init__ like that, your PointOnUnitCircle has a different interface to Point (as it takes angle rather than x, y) and therefore shouldn't really be a sub-class of it. How about something like:

class PointOnUnitCircle(Point):

    def __init__(self, x, y):
        if not self._on_unit_circle(x, y):
            raise ValueError('({}, {}) not on unit circle'.format(x, y))
        super(PointOnUnitCircle, self).__init__(x, y)

    @staticmethod
    def _on_unit_circle(x, y):
        """Whether the point x, y lies on the unit circle."""
        raise NotImplementedError

    @classmethod
    def from_angle(cls, angle):
        return cls.from_mag_angle(1, angle)

    @classmethod
    def from_mag_angle(cls, mag, angle):  
        # note that switching these parameters would allow a default mag=1
        if mag != 1:
            raise ValueError('magnitude must be 1 for unit circle')
        return super(PointOnUnitCircle, cls).from_mag_angle(1, angle)

This keeps the interface the same, adds logic for checking the inputs to the subclass (once you've written it!) and provides a new class method to easily construct a new PointOnUnitCircle from an angle. Rather than

p3 = PointOnUnitCircle(pi/4)

you have to write

p3 = PointOnUnitCircle.from_angle(pi/4)
jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
0

You can override the subclass's __new__ method to construct instances from the superclass's alternate constructor as shown below.

import math


class Point(object):

    def __init__(self, x, y):
        self.x = x
        self.y = y

    @classmethod
    def from_polar(cls, radius, angle):
        x = radius * math.cos(angle)
        y = radius * math.sin(angle)
        return cls(x, y)


class PointOnUnitCircle(Point):

    def __new__(cls, angle):
        point = Point.from_polar(1, angle)
        point.__class__ = cls
        return point

    def __init__(self, angle):
        pass

Note that in __new__, the line point = Point.from_polar(1, angle) cannot be replaced by point = super().from_polar(1, angle) because whereas Point sends itself as the first argument of the alternate constructor, super() sends the subclass PointOnUnitCircle to the alternate constructor, which circularly calls the subclass's __new__ that calls it, and so on until a RecursionError occurs. Also note that even though __init__ is empty in the subclass, without overriding __init__ in the subclass, the superclass's __init__ would automatically be called immediately after __new__, undoing the alternate constructor.

Alternatively, some object designs are simpler with composition than with inheritance. For example, you could replace the above PointOnUnitCircle class without overriding __new__ with the following class.

class UnitCircle:

    def __init__(self, angle):
        self.set_point(angle)

    def set_point(self, angle):
        self.point = Point.from_polar(1, angle)
Victor
  • 127
  • 8
  • 1
    This has exactly the problems that my answer discusses. Your *"subclass"* now has a completely different signature to the parent, and if you call `from_polar` on it directly bad things happen. You cannot just use it wherever you used the parent. How awkward the implementation is is suggestive of its being a bad idea. – jonrsharpe Jan 14 '16 at 07:50
  • Good point. I would have to make the `from_polar` into a static method to work around that problem and that might lead to more problems. Forgoing compatibility with `super` and reaching into `__new__` and `__class__` do seem like red flags. I'm glad you pointed out the flaw. Where the subclass only makes sense with the superclass's alternate constructor, it seems best to use composition because even with the wrinkle-free solution you offer, normal construction from the subclass is no longer useful, and in that sense, as you say, "You cannot just use it wherever you used the parent." – Victor Jan 14 '16 at 09:41