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

Provide function for HTML display #755

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 30 additions & 9 deletions nengo_gui/components/htmlview.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import collections

import nengo

from nengo_gui.components.component import Component


Expand All @@ -10,31 +12,50 @@ class HTMLView(Component):

def __init__(self, obj):
super(HTMLView, self).__init__()
self.obj = obj
self.obj_output = obj.output
self.data = collections.deque()

self.obj = obj
self.obj_output = self.obj.output
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a difference between self.obj_output and self.obj.output? There's one line self.obj.output = self.obj_output in a function which is doing the opposite assignment and I don't understand why. The two variables don't seem to be kept in sync and it seems to be random which one is used where.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So self.obj_output always represents the original node function. If not callable_html, then we need to keep this around since we overwrite the node output, and need to be able to set it back in remove_nengo_objects. It's not random which is used where: self.obj.output is only used when we're setting or re-setting the node output, and self.obj_output is used whenever we want to call the old node function. This is consistent with how this class currently works in master.

self.callable_html = callable(obj.output._nengo_html_)
if self.callable_html:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that this attribute is checked in almost every method makes me wonder if this might be a case for polymorphism. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Maybe provide a helper function that automatically instantiates the right class. Something like

def HTMLView(obj):
    if callable(obj.output._nengo_htlm):
        return CallableHTMLView(obj)
    else
        return StaticHTMLView(obj)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this out. In some ways it's cleaner, but the inheritance does add it's own form of cognitive complexity. Frankly, I'm just not sure if this class is large enough to really benefit from inheritance.

And now, I'm getting an error: ConfigError: Type 'CallableHTMLView' is not set up for configuration. Call configures('CallableHTMLView') first.

I've put the code in the html-function-refactor branch. In the spirit of reviewers making the changes, you guys are welcome to run with that and try to figure out the error. But I don't have time to sink into that, and I think that what I have here is fine (or at least not worth the effort).

assert self.obj.size_out > 0
else:
assert callable(self.obj_output)

def attach(self, page, config, uid):
super(HTMLView, self).attach(page, config, uid)
self.label = page.get_label(self.obj)

def add_nengo_objects(self, page):
with page.model:
if self.callable_html:
with page.model:
self.node = nengo.Node(
self.gather_data, size_in=self.obj.size_out)
self.conn = nengo.Connection(self.obj, self.node, synapse=None)
else:
self.obj.output = self.gather_data

def remove_nengo_objects(self, page):
self.obj.output = self.obj_output
if self.callable_html:
page.model.connections.remove(self.conn)
page.model.nodes.remove(self.node)
else:
self.obj.output = self.obj_output

def gather_data(self, t, *x):
value = self.obj_output(t, *x)
data = '%g %s' % (t, self.obj_output._nengo_html_)
self.data.append(data)
if self.callable_html:
value = None
html = self.obj_output._nengo_html_(t, *x)
else:
value = self.obj_output(t, *x)
html = self.obj_output._nengo_html_

self.data.append('%g %s' % (t, html))
return value

def update_client(self, client):
while len(self.data) > 0:
item = self.data.popleft()
client.write_text(item)
client.write_text(self.data.popleft())

def javascript(self):
info = dict(uid=id(self), label=self.label)
Expand Down
2 changes: 1 addition & 1 deletion nengo_gui/components/netgraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ def get_extra_info(self, obj):
isinstance(obj.output, OverriddenOutput)
and obj.output.base_output is None):
info['passthrough'] = True
if callable(obj.output) and hasattr(obj.output, '_nengo_html_'):
if hasattr(obj.output, '_nengo_html_'):
info['html'] = True
info['dimensions'] = int(obj.size_out)
elif isinstance(obj, nengo.Ensemble):
Expand Down