Skip to content
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

Using numbers as keys in graphology results in some unexpected behaviour in SigmaJS #1454

Open
arthur-bigeard-gdotv opened this issue Aug 15, 2024 · 4 comments

Comments

@arthur-bigeard-gdotv
Copy link

Guidelines

Please note that GitHub issues are only meant for bug reports/feature requests.
If you have questions on how to use Sigma.js, please ask on StackOverflow instead of creating an issue here.

To help us understand your issue, please specify important details, primarily:

  • Sigma.js version : 3.0.0-beta.26
  • Graphology version : 0.25.4
  • Operating system: OS agnostic
  • Web browser: Browser agnostic

To reproduce the issue, create a SigmaJS graph display with numeric keys for nodes and edges in graphology. This causes SigmaJS to behave in sometimes hard to spot but definitely glitchy ways, particularly when handling node labels and hover node labels.
In the screenshot below we can see this is causing both the hover label and the regular label to get superimposed, and the hover label state appears to be stuck once it's been set once:
image

It's hard to describe the glitchy behavior in detail or how to reproduce it but it only occurs with numeric keys for edges/nodes. I've noticed that graphology converts all keys to strings (https://graphology.github.io/design-choices.html#keys) and that sigmajs keeps some references to renderer programs indexed by element key. With some inspection you can see for instance clear discrepancies between how certain fields in sigma (displayedNodeLabels, highlightedNodes) represent those keys, as shown in the screenshot below:

image

It looks like displayedNodeLabels might be at fault here as it represents those node/edge keys as strings regardless of their original type, whereas other fields such as highlightedNodes/nodeIndices appear to retain their original type.

@jacomyal
Copy link
Owner

I can't reproduce it locally, by just naively switching my IDs to numbers in some graph. I can imagine some issues occurring when using different numbers that are cast to the exact same string. Could you provide some data sample?

@arthur-bigeard-gdotv
Copy link
Author

arthur-bigeard-gdotv commented Aug 27, 2024

I've created a sandbox where you can definitely see the same type mismatch between displayedNodeLabels and the rest of the id indexed fields of the renderer here

Note that to see the issue in the console you'll need to open the console in the preview where I'm console.log-ing the renderer object, in which the id types will be mismatched.

This specific example won't show any weird behavior, I believe what's causing the issue in my case is the use of reducers that set node values such as forceLabel/hideLabel/highlighted. What makes me think there's a definitely an issue with this type mismatch is that when using string IDs, none of this is a problem.

[EDIT] I'm noticing in my original screenshot I'm slightly misreading the console in the sense that technically dictionary keys are just going to be strings regardless, and won't be displayed with quotes. I'm still left wondering then where the issue might be happening, my only observation is that this happens with number id's and not strings.

@arthur-bigeard-gdotv
Copy link
Author

Good news - I've figured out what the problem is. The updated refresh function's partialGraph field accepts direct input from the user that isn't sanitized or rather checked against Sigma's internal nodesDataCache.
This means in my case that when doing partial refreshes, I'm passing element id's in their original format (number) whilst both graphology and sigma internally convert those to strings.
The exact issue happening is that partialGraph updates calling updateNode(node) will insert numbers in the highlightedNodes Set rather than strings, hence the mismatch.
I'm unsure there's actually anything to be done on Sigma's side, from my perspective I can simply force everything to string, but let me know if you think different. It's a slight gotcha I guess!

@efwb001
Copy link

efwb001 commented Sep 2, 2024

Forcing to string seems helpful, I too use numbers as keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants