-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix: separate series subgraphs by index name #112
Conversation
Yes, all must be the same. |
src/sciline/visualize.py
Outdated
return name.split('[')[0] | ||
sgname = name.split('[')[0] | ||
if sgname == 'Series': | ||
return name.partition('[')[-1].partition(']')[0] |
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.
A comment would be useful here.
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've added a comment but I'm not sure it's clear and accurate. Do you have a better suggestion?
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.
My point was: It is unclear to a superficial reader what "name.partition('[')[-1].partition(']')[0]" does, can you maybe add an example of what it is parsing/partitioning, to explain this?
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.
Why of there are some nested [...]
, does this still work?
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.
No it doesn't :/ (fixed now)
bdcae87
to
3b0ad13
Compare
Fixes #104
Here's how the graph looks after the change:
I'm still a bit unsure what the correct behavior is here.
When should two
Series[ ... ]
nodes be collected in a subgraph?Is it when all types inside
[ ... ]
are the same?