Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AttributeError when comparing nodes #433

Closed
m-reichle opened this issue Jul 4, 2023 · 6 comments · Fixed by #434
Closed

AttributeError when comparing nodes #433

m-reichle opened this issue Jul 4, 2023 · 6 comments · Fixed by #434
Labels
bug Something isn't working

Comments

@m-reichle
Copy link
Contributor

Hi,

the new version of Abstract Node.__eq__() has me run into errors with custom nodes that do not implement the attributes tested in __eq__. I'd say if the AbstractNode class has a method that assumes the existence of attributes, those should either have defaults or there should be a hasattr() beforehand. Not sure which would make more sense here.

I'd propose verifying this in a very basic test such as this.

from lona.html.abstract_node import AbstractNode

an1 = AbstractNode()
an2 = AbstractNode()

assert an1 == an1
assert an1 != an2
@fscherf
Copy link
Member

fscherf commented Jul 4, 2023

Hi @m-reichle,

lona.html.abstract_node.AbstractNode is not intended to be inherited from in application code. lona.html.Node is what you are presumably searching for. Or is there any reason for you to use the abstract class instead?

@m-reichle
Copy link
Contributor Author

m-reichle commented Jul 5, 2023

I'm inheriting from Widget, which in turn inherits from AbstractNode.

Widget behaves the same way.

from lona.html.widget import Widget
w1 = Widget()
w2 = Widget()
w1 == w1
---------------------------------------------------------------------------
AttributeError: 'Widget' object has no attribute 'namespace'

@fscherf
Copy link
Member

fscherf commented Jul 5, 2023

I see. Im this case this should be fixed. I would recommend using lona.html.Node instead though since lona.html.Widget is deprecated and will be removed in Lona 2. lona.html.Node has all fearures of lona.html.Widget now, so porting is pretty fast-forward

@fscherf fscherf added the bug Something isn't working label Jul 5, 2023
@fscherf
Copy link
Member

fscherf commented Jul 6, 2023

@m-reichle: I opened #434 to fix this issue

@fscherf
Copy link
Member

fscherf commented Jul 10, 2023

@m-reichle: can you verify if the PR fixes the problem?

@m-reichle
Copy link
Contributor Author

Will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants