-
Notifications
You must be signed in to change notification settings - Fork 3
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
Basic Visualisation #15
base: master
Are you sure you want to change the base?
Conversation
@danpalmer were there any particular things you thought that the example visualisation should do? The |
The main things I wanted to do were around readability. I remember it being pretty basic and not very clear. Destinations, triggers, defaults, etc would be good to get on there, maybe the field used to control the destination selection, etc. |
@danpalmer since we discovered that |
This renders in JS into a canvas
Cytoscape is apparently fine with this
51fa9ac
to
a52bc9b
Compare
e9a33c2
to
51b6b15
Compare
51b6b15
to
345837a
Compare
This avoids needing to worry about escaping the content of our JSON.
from routemaster.config import Action, StateMachine | ||
|
||
|
||
def nodes_for_cytoscape( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how much I like the naming being specific to the library we're using on the front end.
On the one hand, this looks like a specific format that wouldn't easily be used in other tools, but on the other, it feels like we shouldn't be encoding JS dependencies in Python naming like this.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally had the massively more vague name convert_to_network
and change to this since the output format is specific to cytoscape
. I'm not sure whether I like that the name is coupled to the library, however since the data format is pretty much coupled anyway, I think having the name be clear is better than the alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data format could be used by other code paths and funged into whatever format is necessary.
How about state_machine_for_render
?
I'd imagine we might add a label_for_render
in the future too?
This seems to produce a slightly better layout in some cases.
This was disabled because zooming was annoying and the layouts were better without this. Now that zooming is less sensitive, fitting the first view results in a more useful initial view.
While there's not a lot we can usefully assert about the returned content, we can assert that it's the expected type.
This clarifies that it's data not code.
500491d
to
bdaaa35
Compare
bdaaa35
to
277c214
Compare
This PR introduces visualisation of state machines and labels locations in them.
Note: this has been handy for debugging configs but low priority and will likely not be finished and merged before launch.
TODO:
onchange
, etc.)