laike9m / Cyberbrain

Python debugging, redefined.
http://bit.ly/cyberbrain-features
MIT License
2.51k stars 159 forks source link

When dragging a node, all related nodes are all moved as whole #46

Open no1xsyzy opened 4 years ago

no1xsyzy commented 4 years ago

"related nodes" means all nodes that show value when hovering the dragged node.

The outcome includes nearly impossible for arranging node graph order in loops, since all values are all bind together.

Example code:

from cyberbrain import trace

@trace
def fib(n):
    a = 0
    b = 1
    for _ in range(n):
        t = b
        b = a + b
        a = t
    return b

if __name__ == '__main__':
    fib(10)
laike9m commented 4 years ago

Do you mind posting a screenshot, ideally a gif? Thanks.

no1xsyzy commented 4 years ago

abc

laike9m commented 4 years ago

To be honest, I don't have full control over visualization, including animation/interaction. I'm using vis-network as the visualization framework, and as a result, we're limited by the capabilities it provides. I've been tunning the parameters for a long time, and there's no perfect solution.

The above is just to give you an idea of how complex visualization is, but I surely want to make it look better as long as I can. Seems you want to drag a to the far right, may I ask why you need that? The graph looks fine to me.

laike9m commented 4 years ago

However I do notice a bug: there should be only one _ node, instead of 3.

no1xsyzy commented 4 years ago

Once the third t got between the first and the second after I had shaken it too hard. That sucks.

I think there should be triple _-s since I edited loop var from 0 through 2. _-s' values are 0, 1, 2. In case you didn't notice, I uses _ for loop that don't actually use that variable (borrowed from go language's convention).

laike9m commented 4 years ago

They're indeed bugs. Not only that, there shouldn't be that many nodes of a and b actually(same for #47), cause the graph is meant to show only one iteration at a time.

I've come with an idea for refactoring, but it's gonna take some time. Once done, the issue here will at least be mitigated.

no1xsyzy commented 4 years ago

Okay, now I wonder how you expect to show that the value is derived from value from former iteration. I think it can be a solution or at least a workaround to destroy and recreate the whole graph.

laike9m commented 4 years ago

Okay, now I wonder how you expect to show that the value is derived from value from former iteration. I think it can be a solution or at least a workaround to destroy and recreate the whole graph.

That is a pain point. The value derived from previous iterations will miss the links to it's sources unfortunately. I do plan to fix it though, check out #19.

The refactoring will do what you suggest by recreating the entire graph, because I believe it's more robust than the current implementation, which removes nodes in the old iteration, then add new nodes.