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

Exception in widget __del__ methods in Python 3.4. #5282

Closed
takluyver opened this issue Mar 5, 2014 · 4 comments · Fixed by #5283
Closed

Exception in widget __del__ methods in Python 3.4. #5282

takluyver opened this issue Mar 5, 2014 · 4 comments · Fixed by #5283
Labels
Milestone

Comments

@takluyver
Copy link
Member

When I run the html section of the test suite on Python 3.4, I see a lot of exceptions in __del__ methods. Python 3.4 does have changes to object finalisation, which might be responsible for this. I'm looking into this, but @jdfreder, is there anything that springs to mind?

Here's an example. I see the same traceback with a load of different classnames - TextWidget, FloatSliderWidget, DropdownWidget...

Exception ignored in: <bound method ContainerWidget.__del__ of <IPython.html.widgets.widget_container.ContainerWidget object at 0x7f346a61f898>>
Traceback (most recent call last):
  File "/home/takluyver/.local/lib/python3.4/site-packages/IPython/html/widgets/widget.py", line 130, in __del__
    self.close()
  File "/home/takluyver/.local/lib/python3.4/site-packages/IPython/html/widgets/widget.py", line 175, in close
    self._comm.close()
AttributeError: 'NoneType' object has no attribute 'close'
@jdfreder
Copy link
Member

jdfreder commented Mar 5, 2014

Nothing springs to mind, it looks like the _comm is getting cleaned up before the widget though.

@takluyver
Copy link
Member Author

I think the problem is that if the widget's comm attribute is never accessed, then its _comm is never set. @minrk suggested that Widget.closed should start as True, and be set to False when the comm is created. I think we don't actually need the closed attribute at all, and we can just check self._comm is None where we currently use it.

Does that make sense? Do we want closed to remain as a public API, in which case I'll turn it into a property? Or shall I just get rid of it entirely?

@takluyver
Copy link
Member Author

Getting rid of closed does seem to fix it. I'll make a PR.

takluyver added a commit to takluyver/ipython that referenced this issue Mar 5, 2014
@minrk minrk added this to the 2.0 milestone Mar 5, 2014
@jdfreder
Copy link
Member

jdfreder commented Mar 5, 2014

Does that make sense? Do we want closed to remain as a public API, in which case I'll turn it into a property? Or shall I just get rid of it entirely?

That makes sense. I think I've only ever seen one case where someone was thinking of using closed, but the PR itself was closed (no pun intended). +1 for reducing the API's size.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants