search

cytoscape / ipycytoscape

A Cytoscape Jupyter widget
https://ipycytoscape.readthedocs.io/en/master/
BSD 3-Clause "New" or "Revised" License
264 stars 62 forks source link

Date is not JSON serializable #298

Closed pwrose closed 2 years ago

pwrose commented 2 years ago

Bug report

Bug summary

If a node_attribute, e.g. a Date object, is not JSON serializable, I get the following error:

      1 widget1 = ipycytoscape.CytoscapeWidget()
----> 2 widget1.graph.add_graph_from_neo4j(subgraph1)

~/miniconda3/envs/dkg/lib/python3.7/site-packages/ipycytoscape/cytoscape.py in add_graph_from_neo4j(self, g)
    739             # create node
    740             node_instance = Node()
--> 741             _set_attributes(node_instance, node_attributes)
    742             node_list.append(node_instance)
    743 

~/miniconda3/envs/dkg/lib/python3.7/site-packages/ipycytoscape/cytoscape.py in _set_attributes(instance, data)
    250             setattr(instance, k, v)
    251         else:
--> 252             instance.data[k] = v

...

~/miniconda3/envs/dkg/lib/python3.7/site-packages/jupyter_client/jsonutil.py in json_default(obj)
    124         return float(obj)
    125 
--> 126     raise TypeError("%r is not JSON serializable" % obj)
    127 
    128 

TypeError: Date(2020, 11, 6) is not JSON serializable

Code for reproduction

# Paste your code here
#
#

Actual outcome

Expected outcome

Version Info

marimeireles commented 2 years ago

I'm thinking if all data shouldn't be serialized? What if you try to convert it with dumps? Idk if this make sense, sorry if it doesn't, but I'm struggling to find a good generic solution for this.

You think we should convert it always before calling this method add_graph_from_neo4j, for example? Or do you have any input on what should we do here?

Thanks for opening the issue! Never thought about this before.

marimeireles commented 2 years ago

Ah, this issue was fixed by your https://github.com/cytoscape/ipycytoscape/pull/297, correct? Sorry for the confusion :)

pwrose commented 2 years ago

Yes, the pr fixes the issue in a general way.

On Tue, Jan 18, 2022 at 11:28 AM Mariana Meireles @.***> wrote:

Ah, this issue was fixed by your PR, correct? Sorry for the confusion :)

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you authored the thread.Message ID: @.***>