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

Fix target/source keymapping #402

Merged
merged 6 commits into from
Jun 28, 2022

Conversation

thomcom
Copy link
Contributor

@thomcom thomcom commented Jun 28, 2022

Previously, the index value referred to by a graphology graph edges list in the source and target values was being used to lookup nodes according to their position in the nodes array. This is incorrect.

This bugfix creates the key=>index mapping so that source and target refer to nodes by their key entry, not their list position.

This bugfix also moves the default index.js execution of @rapidsai/demo-api-server to port 3010, compatible with the sigma.js demo.

Finally this bugfix adds .setNullMask after the getJSONObject calls, as spurious entries in the nullmask are occurring there since the original merge #392

@AjayThorve
Copy link
Member

@thomcom not sure if it's because of this PR or the previous one, but I cannot run the demo locally:

Error: Cannot find module '../util/schema.js'
Require stack:
- /opt/rapids/node/modules/demo/api-server/routes/root.js

seems like ./util/schema.js is missing?

@thomcom
Copy link
Contributor Author

thomcom commented Jun 28, 2022

@thomcom not sure if it's because of this PR or the previous one, but I cannot run the demo locally:

Error: Cannot find module '../util/schema.js'
Require stack:
- /opt/rapids/node/modules/demo/api-server/routes/root.js

seems like ./util/schema.js is missing?

Oh yeah, that's extremely important, thanks for the catch.

@thomcom
Copy link
Contributor Author

thomcom commented Jun 28, 2022

@thomcom not sure if it's because of this PR or the previous one, but I cannot run the demo locally:

Error: Cannot find module '../util/schema.js'
Require stack:
- /opt/rapids/node/modules/demo/api-server/routes/root.js

seems like ./util/schema.js is missing?

Oh yeah, that's extremely important, thanks for the catch.

Also @trxcllnt this makes me pretty stressed out because I see that we're not running any tests as part of CI/PR approval?

@thomcom thomcom merged commit 5f3833b into rapidsai:main Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants