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

add constructor to main widget #258

Closed vaniisgh closed 2 years ago

vaniisgh commented 3 years ago

Hey, This issue ( #94 ) looked interesting and beginner-friendly, I just wanted to know if this was a good start.

github-actions[bot] commented 3 years ago

Binder :point_left: Launch a binder notebook on branch vaniisgh/ipycytoscape/master

ianhi commented 3 years ago

Hi @vaniisgh thanks for taking this on! This looks like a good start but there are a few things to clean up.

TODO:

  1. Add some examples documenting this
  2. Add a docstring to __init__ with parameters section
  3. Cleanup checking for string in the from_json method (I'll add an inline review) (edit I think you did this correctly and I was just reading the code too fast - my bad!)
  4. Add a check for neo4j graphs
  5. Change the name from graph_file to something more general.
    • It's not necessarily a file that user will be passing in.
  6. Make the formatter happy by running black .

Also looking at the test failures there are unfortunately several due to neo4j which aren't your fault. This makes it trickier to see if your are causing issues but you can look more closely by clicking on the details next to the failing test.

vaniisgh commented 3 years ago

hey thank you so much for the feedback! I tried to take the things you mentioned into account and made a few changes. Though, I still think I might need to revise my work a bit. Also, I apologize forgot to rebase my work on the current master I hope I did it correctly now.

vaniisgh commented 3 years ago

I think it should be better now :)

ianhi commented 3 years ago

Thanks for the ping! This slipped through the cracks for me - can look later

vaniisgh commented 2 years ago

@ianhi should i close this ?

ianhi commented 2 years ago

Thank you @vaniisgh !! This is a great addition and we can always iterate more as needed.

ianhi commented 2 years ago

@ianhi should i close this ?

Definitely not that - I'm really sorry about the long periods of non-responsiveness that's totally on me. I really appreciate you coming and putting the effort to make a positive change. I can't promise to be perfect, but I can promise that I will at least try to be more honest myself about what I can claim to do. Also don't be scared to ping more and more often (though ideally course you shouldn't have to :/ )

ianhi commented 2 years ago

This should be live in version 1.3.0 soon!