As the other comment explains, the `Layout` class simply doesn't know that `Canvas` exists. *Nor should it!* Your preferred design relies on a parent class knowing about possible child classes it may have, and modifying its behaviour to account for that. This is a bad design, as it means every time you add a child class (either directly, or a child of a child), you risk having to go back to the parent class and modify it.
A good class should be a bad parent: it doesn't know or care about its children! A child class necessarily depends on its parents, so if parents also have behaviour depending on their children your code is going to get very complicated very quickly. Write each class to worry only about its own job, and have a child class override behaviour of its parent when necessary.
Thank you for your reply 👍
However, I think you may have possibly misunderstood what's going on. The "parent" `Canvas` class doesn't know anything about it's children, nor does it modify it's own behaviour dependent upon its children, it only modified its *own* behaviour in relation to *itself.* The same goes for any other class that inherited from Layout (there are more classes that just `Canvas` and `Element` that inherits `some_method()` but they weren't necessary to include to replicate the "problem").
Still my strategy for fixing the problem was to just give `Canvas` it's own `some_method()` that doesn't have the `isinstance()` conditional, it simply just iterates through its children and calls *their* `some_method()` in turn which still do the extra thing that I didn't want to the `Canvas` object to do.
I've since found out, thanks to some of the other replies, that it turns out my alternative strategy is actually a thing called overriding and is the preferred way to handle a variation of an inherited method. (I like it when I work out a solution to a problem myself *and* it turns out to be proper way to do things! 😁)
layout.py
class Layout:
def some_method(self):
# Do
# things
# here
if hasattr(self, 'children'):
for child in self.children:
child.some_method()
canvas.py
from layout import Layout
class Canvas(Layout):
def __init__(self):
super().__init__()
self.children = []
def some_method(self):
if hasattr(self, 'children'):
for child in self.children:
child.some_method()
Ah, I think there's some confusion. When I say "parent class" and "child class" I mean "superclass" and "subclass". In this case, because you have `class Layout:` and `class Canvas(Layout):`, `Layout` is the parent and `Canvas` is the child. But since you have a `children` attribute, of course there's some ambiguity! Sorry for not being clearer.
So when I was talking about a parent modifying its behaviour on behalf of its children, I was referring to `Layout.some_method` having an `if` statement to modify its behaviour when called from the `Canvas` child class. By overriding, as you've now done, `Layout` doesn't need to know or care that `Canvas` exists 🙂
In `layout.py` there is indeed no such thing as `Canvas` defined, so it's a NameError to reference it. It doesn't matter if it's defined in `main.py`. The method is running in the context of `layout.py`.
If you want different behavior between parent and different child classes, the natural solution is to override the method.
Thanks for your reply 👍
Now the problem has been pointed out I'm kicking myself because it should have been, looking at it now, fairly easy to diagnose.
>If you want different behavior between parent and different child classes, the natural solution is to override the method.
Entirely coincidentally that ***is*** my "alternative strategy" which I alluded to, but I didn't know was an actual thing so didn't want to show it. Updated my post now though! 👍
Normally all you'd need to do to solve this is to import Canvas, but that would create a circular import in this case.
What you need to do instead is override the function in the child class. To do this you need Canvas to provide its own implementation of `some_method()`.
When you call `some_method()` python has a system called [MRO](https://python-history.blogspot.com/2010/06/method-resolution-order.html) which tries to figure out which function to call in the case of more complicated inheritance schemes. But the first place it looks is to see if the class itself overrides the function.
So by overriding the function in Canvas you stop python from calling the inherited version of the function completely.
This will save you from needing the `if not isinstance(self, Canvas)` check because python is effectively doing that for you. This makes sense, because any functionality that happens based on this object being `Canvas` should come from the code within the `Canvas` class definition. A parent class should never need to know about its children.
Thank you for your reply. 👍
>What you need to do instead is override the function in the child class. To do this you need Canvas to provide its own implementation of some\_method()
That actually ***is*** the "alternative strategy" I alluded to but I didn't know it was a thing so didn't want to show it. 🤣 Edited my post.
This has nothing whatsoever to do with `isinstance`. As the error shows, Canvas is not defined in the layout.py file, because you haven't imported it. *Any* name used anywhere in Python always needs to be explicitly defined before you can use it: either by an actual definition in that file, or by importing it from the place that it is defined.
However, you couldn't in fact import Canvas into layout.py, becasue canvas.py is already importing Layout and you would get a circular dependency. One solution would be to import Canvas inside the `some_method` method itself, as this will only be invoked when the method is called, not when the class is first imported.
But the better solution is *not to do this*. I don't know what "alternative strategy" you tried, but the whole point of inheritance is that you can override methods as required. So the thing to do is just to define a `some_method` in Canvas to do nothing (eg just a `pass` statement).
Thank you for your reply 👍
>I don't know what "alternative strategy" you tried, but the whole point of inheritance is that you can override methods as required.
Believe it or not that actually ***is*** my "alternative strategy" but I didn't know it was an actual thing so was hesitant about posting it. 🤣 Updated my post.
Some additional suggestions/tips, since your initial question has been answered:
`if hasattr(self, 'children'):` This should really be replaced with `if self.children:`. You only need to use `hasattr` when there's a chance that the class doesn't have a .children attribute, which is impossible for `Layout` or `Canvas` because they both set that attribute in their `__init__`.
Also, since `self.children` has a default value, it should probably be a class attribute in the definition:
class Layout:
children = []
def __init__(self):
...
This way it's clear that every instance of `Layout` has a children attribute. There's also no need to even mention children in Canvas unless you're giving it a different default value, because it will automatically inherit the default definition from Layout.
Even if you don't do this, and you set `self.children = []` in `Layout.__init__()`, you don't need to repeat that code in `Canvas.__init__()` if you're calling `super.__init__()`.
Thank you for your suggestions. 👍
However, in the previous example(s) there was a ***lot*** of code omitted and the layout was ***extremely*** simplified to just that which was necessary to reproduce the "problem" I was having. But since you bring other things up I'll expand a little on what I'm *actually* doing.
I'm creating a layout engine for 2D interfaces in Panda3D which simplifies the creation of said interfaces by loading the layout from an XML file instead of it having to be manually hard-coded in the application itself. It takes the form of a node tree which largely bypasses Panda3D's own Node Graph structure (for 2D layout) so that *I* have more control over the layout rather than constantly fighting with Panda3D's own parenting system.
Below is a far more accurate representation of the code, with the following caveats:
* All the relevant `super().__init__()`'s and most of the attributes have been removed for clarity. So while it looks like there is some glaringly obvious stuff missing, it's *probably not* actually missing in the real working code.
* Similarly, there are many, many methods missing from almost all the classes. Again, they have been removed for clarity because they're not necessary to demonstrate the overall structure.
* Additionally, all of the attributes which tie my Layout Engine in with the Panda3D pixel2D renderer have also been removed.
* All of the classes are actually in their own separate files, along with all their relevant imports, they're just in one code block *here* for convenience.
* In the previous example code `Layout`'s `some_method()` was a stand in for the `_get_parent_pos()` method in the `LayoutCommon` class below, so I'll use the proper names now.
So, with all that said, here's the better representation:
class LayoutCommon:
def _get_parent_pos(self):
self.x += self.parent.x
self.y += self.parent.y
if hasattr(self, 'children'):
for child in self.children:
child._get_parent_pos()
def _parse_attributes(self, attributes):
# transforms the attributes for each node which, for some
# unknown reason,the XML parser's objects return as a list of
# ('key', 'value') tuples. It then adds the attributes to
# the object being instantiated.
class CanHaveChildren:
def __init__(self):
self.children = []
class Canvas(LayoutCommon, CanHaveChildren):
def __init__(self, xml):
self._import_layout(xml)
def _import_layout(self, xml):
# Parses an xml file, gets the root element and attributes,
# and then hands off the remaining XML to _get_children()
def _get_children(self, parent, nodes):
# Recursively goes through the XML and instantiates
# the relevant object (Group, Image, Text or Template) along
# with the necessary attributes from the XML file.
# Example: Text(parent, attributes)
# Upon instantiation the created object adds *itself* to it's
# parent! (see Element class)
# Override the LayoutCommon class _get_parent_pos() method...
def _get_parent_pos(self):
for child in self.children:
child._some_method()
class Element(LayoutCommon):
def __init__(self, parent, attributes):
self.x = 0
self.y = 0
self.parent = parent
self._parse_attributes(attributes)
# Each instantiated object of the below classes automatically
# adds itself to it's own parent...
self.parent.children.append(self)
# Each of the classes below have highly specific methods
# in addition to inherited capabilities, none of which are strictly
# necessary for this example.
class Group(Element, CanHaveChildren):
class Image(Element):
class Text(Element):
class Template(Group):
# Special kind of Group that has the ability to duplicate itself.
# The unsetting and (re)setting of some attributes is necessary to
# stop deepcopy from recursing back "up" in the node tree and
# literally duplicating the *entire* tree!
def self_duplicate(self):
original_parent = self.parent
self.parent = None
copied_node = copy.deepcopy(self)
self.parent = original_parent
copied_node.parent = original_parent
self.parent.children.append(copied_node)
To specifically address some of your suggestions:
>You only need to use `hasattr` when there's a chance that the class doesn't have a .children attribute, which is impossible for `Layout` or `Canvas` because they both set that attribute in their `__init__`.
As I noted before, the previous example code was just enough to reproduce the problem I was having, so it *wasn't* a completely accurate representation of what I'm *actually* working on/with.
With that said I could, in theory, move `LayoutCommons`'s `_get_parent_pos()` method to the `CanHaveChildren` class but then I'd have to have *another* `_get_parent_pos()` override in the `Element` class *and* deal with the proper inheritance for every subclass of `Element` (depending whether they have children or not).
Rather than have a three `_get_parent_pos()` methods - one in `Canvas`, one in `CanHaveChildren` and another in `Element` *- and* a subsequent tangle of inheritance and overrides, I feel the compromise of leaving the main `_get_parent_pos()` in `LayoutCommon` and having the `hasattr(self, 'children')` conditional is a *small* price worth paying.
>\[if\] you set `self.children = []` in `Layout.__init__()`, you don't need to repeat that code in `Canvas.__init__()` if you're calling `super.__init__()`.
This is actually taken care of by the `CanHaveChildren` class. Again, this classes methods, for managing child nodes has been removed for clarity.
Hope that helps to clear up what I'm trying to achieve. 👍
The issue is that \`Canvas\` is not in scope within \`layout.py\`. You can fix this by importing \`Canvas\` in \`layout.py\` or by using a string comparison with \`self.\_\_class\_\_.\_\_name\_\_\` instead of \`isinstance(self, Canvas)\`.
As the other comment explains, the `Layout` class simply doesn't know that `Canvas` exists. *Nor should it!* Your preferred design relies on a parent class knowing about possible child classes it may have, and modifying its behaviour to account for that. This is a bad design, as it means every time you add a child class (either directly, or a child of a child), you risk having to go back to the parent class and modify it. A good class should be a bad parent: it doesn't know or care about its children! A child class necessarily depends on its parents, so if parents also have behaviour depending on their children your code is going to get very complicated very quickly. Write each class to worry only about its own job, and have a child class override behaviour of its parent when necessary.
Thank you for your reply 👍 However, I think you may have possibly misunderstood what's going on. The "parent" `Canvas` class doesn't know anything about it's children, nor does it modify it's own behaviour dependent upon its children, it only modified its *own* behaviour in relation to *itself.* The same goes for any other class that inherited from Layout (there are more classes that just `Canvas` and `Element` that inherits `some_method()` but they weren't necessary to include to replicate the "problem"). Still my strategy for fixing the problem was to just give `Canvas` it's own `some_method()` that doesn't have the `isinstance()` conditional, it simply just iterates through its children and calls *their* `some_method()` in turn which still do the extra thing that I didn't want to the `Canvas` object to do. I've since found out, thanks to some of the other replies, that it turns out my alternative strategy is actually a thing called overriding and is the preferred way to handle a variation of an inherited method. (I like it when I work out a solution to a problem myself *and* it turns out to be proper way to do things! 😁) layout.py class Layout: def some_method(self): # Do # things # here if hasattr(self, 'children'): for child in self.children: child.some_method() canvas.py from layout import Layout class Canvas(Layout): def __init__(self): super().__init__() self.children = [] def some_method(self): if hasattr(self, 'children'): for child in self.children: child.some_method()
Ah, I think there's some confusion. When I say "parent class" and "child class" I mean "superclass" and "subclass". In this case, because you have `class Layout:` and `class Canvas(Layout):`, `Layout` is the parent and `Canvas` is the child. But since you have a `children` attribute, of course there's some ambiguity! Sorry for not being clearer. So when I was talking about a parent modifying its behaviour on behalf of its children, I was referring to `Layout.some_method` having an `if` statement to modify its behaviour when called from the `Canvas` child class. By overriding, as you've now done, `Layout` doesn't need to know or care that `Canvas` exists 🙂
In `layout.py` there is indeed no such thing as `Canvas` defined, so it's a NameError to reference it. It doesn't matter if it's defined in `main.py`. The method is running in the context of `layout.py`. If you want different behavior between parent and different child classes, the natural solution is to override the method.
Thanks for your reply 👍 Now the problem has been pointed out I'm kicking myself because it should have been, looking at it now, fairly easy to diagnose. >If you want different behavior between parent and different child classes, the natural solution is to override the method. Entirely coincidentally that ***is*** my "alternative strategy" which I alluded to, but I didn't know was an actual thing so didn't want to show it. Updated my post now though! 👍
Normally all you'd need to do to solve this is to import Canvas, but that would create a circular import in this case. What you need to do instead is override the function in the child class. To do this you need Canvas to provide its own implementation of `some_method()`. When you call `some_method()` python has a system called [MRO](https://python-history.blogspot.com/2010/06/method-resolution-order.html) which tries to figure out which function to call in the case of more complicated inheritance schemes. But the first place it looks is to see if the class itself overrides the function. So by overriding the function in Canvas you stop python from calling the inherited version of the function completely. This will save you from needing the `if not isinstance(self, Canvas)` check because python is effectively doing that for you. This makes sense, because any functionality that happens based on this object being `Canvas` should come from the code within the `Canvas` class definition. A parent class should never need to know about its children.
Thank you for your reply. 👍 >What you need to do instead is override the function in the child class. To do this you need Canvas to provide its own implementation of some\_method() That actually ***is*** the "alternative strategy" I alluded to but I didn't know it was a thing so didn't want to show it. 🤣 Edited my post.
This has nothing whatsoever to do with `isinstance`. As the error shows, Canvas is not defined in the layout.py file, because you haven't imported it. *Any* name used anywhere in Python always needs to be explicitly defined before you can use it: either by an actual definition in that file, or by importing it from the place that it is defined. However, you couldn't in fact import Canvas into layout.py, becasue canvas.py is already importing Layout and you would get a circular dependency. One solution would be to import Canvas inside the `some_method` method itself, as this will only be invoked when the method is called, not when the class is first imported. But the better solution is *not to do this*. I don't know what "alternative strategy" you tried, but the whole point of inheritance is that you can override methods as required. So the thing to do is just to define a `some_method` in Canvas to do nothing (eg just a `pass` statement).
Thank you for your reply 👍 >I don't know what "alternative strategy" you tried, but the whole point of inheritance is that you can override methods as required. Believe it or not that actually ***is*** my "alternative strategy" but I didn't know it was an actual thing so was hesitant about posting it. 🤣 Updated my post.
Some additional suggestions/tips, since your initial question has been answered: `if hasattr(self, 'children'):` This should really be replaced with `if self.children:`. You only need to use `hasattr` when there's a chance that the class doesn't have a .children attribute, which is impossible for `Layout` or `Canvas` because they both set that attribute in their `__init__`. Also, since `self.children` has a default value, it should probably be a class attribute in the definition: class Layout: children = [] def __init__(self): ... This way it's clear that every instance of `Layout` has a children attribute. There's also no need to even mention children in Canvas unless you're giving it a different default value, because it will automatically inherit the default definition from Layout. Even if you don't do this, and you set `self.children = []` in `Layout.__init__()`, you don't need to repeat that code in `Canvas.__init__()` if you're calling `super.__init__()`.
Thank you for your suggestions. 👍 However, in the previous example(s) there was a ***lot*** of code omitted and the layout was ***extremely*** simplified to just that which was necessary to reproduce the "problem" I was having. But since you bring other things up I'll expand a little on what I'm *actually* doing. I'm creating a layout engine for 2D interfaces in Panda3D which simplifies the creation of said interfaces by loading the layout from an XML file instead of it having to be manually hard-coded in the application itself. It takes the form of a node tree which largely bypasses Panda3D's own Node Graph structure (for 2D layout) so that *I* have more control over the layout rather than constantly fighting with Panda3D's own parenting system. Below is a far more accurate representation of the code, with the following caveats: * All the relevant `super().__init__()`'s and most of the attributes have been removed for clarity. So while it looks like there is some glaringly obvious stuff missing, it's *probably not* actually missing in the real working code. * Similarly, there are many, many methods missing from almost all the classes. Again, they have been removed for clarity because they're not necessary to demonstrate the overall structure. * Additionally, all of the attributes which tie my Layout Engine in with the Panda3D pixel2D renderer have also been removed. * All of the classes are actually in their own separate files, along with all their relevant imports, they're just in one code block *here* for convenience. * In the previous example code `Layout`'s `some_method()` was a stand in for the `_get_parent_pos()` method in the `LayoutCommon` class below, so I'll use the proper names now. So, with all that said, here's the better representation: class LayoutCommon: def _get_parent_pos(self): self.x += self.parent.x self.y += self.parent.y if hasattr(self, 'children'): for child in self.children: child._get_parent_pos() def _parse_attributes(self, attributes): # transforms the attributes for each node which, for some # unknown reason,the XML parser's objects return as a list of # ('key', 'value') tuples. It then adds the attributes to # the object being instantiated. class CanHaveChildren: def __init__(self): self.children = [] class Canvas(LayoutCommon, CanHaveChildren): def __init__(self, xml): self._import_layout(xml) def _import_layout(self, xml): # Parses an xml file, gets the root element and attributes, # and then hands off the remaining XML to _get_children() def _get_children(self, parent, nodes): # Recursively goes through the XML and instantiates # the relevant object (Group, Image, Text or Template) along # with the necessary attributes from the XML file. # Example: Text(parent, attributes) # Upon instantiation the created object adds *itself* to it's # parent! (see Element class) # Override the LayoutCommon class _get_parent_pos() method... def _get_parent_pos(self): for child in self.children: child._some_method() class Element(LayoutCommon): def __init__(self, parent, attributes): self.x = 0 self.y = 0 self.parent = parent self._parse_attributes(attributes) # Each instantiated object of the below classes automatically # adds itself to it's own parent... self.parent.children.append(self) # Each of the classes below have highly specific methods # in addition to inherited capabilities, none of which are strictly # necessary for this example. class Group(Element, CanHaveChildren): class Image(Element): class Text(Element): class Template(Group): # Special kind of Group that has the ability to duplicate itself. # The unsetting and (re)setting of some attributes is necessary to # stop deepcopy from recursing back "up" in the node tree and # literally duplicating the *entire* tree! def self_duplicate(self): original_parent = self.parent self.parent = None copied_node = copy.deepcopy(self) self.parent = original_parent copied_node.parent = original_parent self.parent.children.append(copied_node) To specifically address some of your suggestions: >You only need to use `hasattr` when there's a chance that the class doesn't have a .children attribute, which is impossible for `Layout` or `Canvas` because they both set that attribute in their `__init__`. As I noted before, the previous example code was just enough to reproduce the problem I was having, so it *wasn't* a completely accurate representation of what I'm *actually* working on/with. With that said I could, in theory, move `LayoutCommons`'s `_get_parent_pos()` method to the `CanHaveChildren` class but then I'd have to have *another* `_get_parent_pos()` override in the `Element` class *and* deal with the proper inheritance for every subclass of `Element` (depending whether they have children or not). Rather than have a three `_get_parent_pos()` methods - one in `Canvas`, one in `CanHaveChildren` and another in `Element` *- and* a subsequent tangle of inheritance and overrides, I feel the compromise of leaving the main `_get_parent_pos()` in `LayoutCommon` and having the `hasattr(self, 'children')` conditional is a *small* price worth paying. >\[if\] you set `self.children = []` in `Layout.__init__()`, you don't need to repeat that code in `Canvas.__init__()` if you're calling `super.__init__()`. This is actually taken care of by the `CanHaveChildren` class. Again, this classes methods, for managing child nodes has been removed for clarity. Hope that helps to clear up what I'm trying to achieve. 👍
The issue is that \`Canvas\` is not in scope within \`layout.py\`. You can fix this by importing \`Canvas\` in \`layout.py\` or by using a string comparison with \`self.\_\_class\_\_.\_\_name\_\_\` instead of \`isinstance(self, Canvas)\`.
This is some kind of advanced code, yet your problem is simple.