Skip to content

Conversation

@jamesmcclain
Copy link

@jamesmcclain jamesmcclain commented Apr 11, 2017

This resolves the following issue that I observed on Python 3:

[IPKernelApp] ERROR | Exception opening comm with target: geonotebook
Traceback (most recent call last):
  File "/usr/lib/python3.4/site-packages/ipykernel/comm/manager.py", line 90, in comm_open
    f(comm, msg)
  File "/home/hadoop/.local/lib/python3.4/site-packages/geonotebook/kernel.py", line 534, in handle_comm_open
    "data": self.geonotebook.get_protocol()
  File "/usr/lib/python3.4/site-packages/ipykernel/comm/comm.py", line 121, in send
    data=data, metadata=metadata, buffers=buffers,
  File "/usr/lib/python3.4/site-packages/ipykernel/comm/comm.py", line 65, in _publish_msg
    content = json_clean(dict(data=data, comm_id=self.comm_id, **keys))
  File "/usr/lib/python3.4/site-packages/ipykernel/jsonutil.py", line 167, in json_clean
    out[unicode_type(k)] = json_clean(v)
  File "/usr/lib/python3.4/site-packages/ipykernel/jsonutil.py", line 167, in json_clean
    out[unicode_type(k)] = json_clean(v)
  File "/usr/lib/python3.4/site-packages/ipykernel/jsonutil.py", line 173, in json_clean
    raise ValueError("Can't clean for JSON: %r" % obj)
ValueError: Can't clean for JSON: dict_values([{'optional': [], 'required': [{'default': False, 'key': 'ann_type'}, {'default': False, 'key': 'coords'}, {'default': False, 'key': 'meta'}], 'procedure': 'add_annotation_from_client'}, {'optional': [], 'required': [{'default': False, 'key': 'x'}, {'default': False, 'key': 'y'}, {'default': False, 'key': 'z'}], 'procedure': 'set_center'}, {'optional': [], 'required': [], 'procedure': 'get_protocol'}, {'optional': [], 'required': [], 'procedure': 'get_map_state'}])

I am willing to make any changes that might be requested/required.

@jamesmcclain jamesmcclain changed the title Cast dict_values To A List Cast dict_values To A list Apr 11, 2017
Copy link
Member

@minrk minrk left a comment

Choose a reason for hiding this comment

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

Thanks! I left a comment about how to test for dict_values.

How did you come across this? It's likely that hitting this means that code elsewhere should change, because dict_values views probably shouldn't be passed to messages at this level.

out = {}
for k,v in iteritems(obj):
out[unicode_type(k)] = json_clean(v)
if str(type(v)) == "<class 'dict_values'>":
Copy link
Member

Choose a reason for hiding this comment

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

dict_values can be checked for with:

if PY3:
    from collections.abc import MappingView

...

if PY3 and isinstance(obj, MappingView):
    obj = list(obj)

and this should be at the top level, not inside the dict case.

@jamesmcclain
Copy link
Author

jamesmcclain commented Apr 18, 2017

How did you come across this? It's likely that hitting this means that code elsewhere should change, because dict_values views probably shouldn't be passed to messages at this level.

I am currently trying to get GeoNotebook running on Python 3; I encountered it there. If there is something that I can do to help detect similar issues I am happy to do so (but as you no-doubt observed, my Python skills are a bit limited).

@jamesmcclain
Copy link
Author

I have attempted to update according to your comment, I hope that I interpreted it correctly.

@jamesmcclain
Copy link
Author

Should I close this PR?

@Carreau
Copy link
Member

Carreau commented May 30, 2017

Sorry it fell under our radar. Appologies, we'll re-have a look.

@jamesmcclain
Copy link
Author

Ah, okay, many thanks.

@minrk minrk added this to the no action milestone Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants