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

Support multiple dataframe input to CE construct #233

Merged
merged 7 commits into from Dec 21, 2022

Conversation

adamnsch
Copy link
Contributor

@adamnsch adamnsch commented Dec 15, 2022

Thank you for your contribution to the Graph Data Science Client project.

Before submitting this PR, please read Contributing to the Neo4j Ecosystem.

Make sure:

  • You signed the Neo4j CLA (Contributor License Agreement) so that we are allowed to ship your code in our library
  • Your contribution is covered by tests

@netlify
Copy link

netlify bot commented Dec 15, 2022

Deploy Preview for neo4j-graph-data-science-client canceled.

Name Link
🔨 Latest commit 32cce59
🔍 Latest deploy log https://app.netlify.com/sites/neo4j-graph-data-science-client/deploys/63a2f7fb758cfd00088cc464

@adamnsch adamnsch force-pushed the multidf-ce-construct branch 2 times, most recently from 290e004 to 1ddd475 Compare December 15, 2022 14:26
@Mats-SX Mats-SX force-pushed the multidf-ce-construct branch 2 times, most recently from 84405aa to bcf0703 Compare December 19, 2022 09:24
@brs96 brs96 force-pushed the multidf-ce-construct branch 8 times, most recently from 3f4dbe3 to cb4141f Compare December 20, 2022 10:01
@brs96 brs96 marked this pull request as ready for review December 20, 2022 10:11
@Mats-SX Mats-SX changed the title WIP: Support multiple dataframe input to CE construct Support multiple dataframe input to CE construct Dec 20, 2022
@Mats-SX Mats-SX changed the title WIP: Support multiple dataframe input to CE construct Support multiple dataframe input to CE construct Dec 20, 2022
@Mats-SX
Copy link
Contributor

Mats-SX commented Dec 20, 2022

The test failure is an unrelated test that is also failing on main, which @DarthMax is working to resolve.

brs96 and others added 5 commits December 21, 2022 11:25
Co-authored-by: Adam Schill Collberg <adam.schill.collberg@protonmail.com>
Co-authored-by: Mats Rydberg <mats@neo4j.org>
Co-authored-by: Mats Rydberg <mats@neo4j.org>
Co-authored-by: Mats Rydberg <mats@neo4j.org>
Co-authored-by: Mats Rydberg <mats@neo4j.org>
@Mats-SX
Copy link
Contributor

Mats-SX commented Dec 21, 2022

Rebased to get the fix for the failing test

Copy link
Contributor Author

@adamnsch adamnsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently I cannot Approve my own PR, so I'll just say it: Approved! ✔️

changelog.md Show resolved Hide resolved
Comment on lines +140 to +141
aligned_node_dfs = self.adjust_node_df(node_dfs, graph_schema)
aligned_rel_dfs = self.adjust_rel_df(relationship_dfs, graph_schema)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
aligned_node_dfs = self.adjust_node_df(node_dfs, graph_schema)
aligned_rel_dfs = self.adjust_rel_df(relationship_dfs, graph_schema)
aligned_node_dfs = self.adjust_node_dfs(node_dfs, graph_schema)
aligned_rel_dfs = self.adjust_rel_dfs(relationship_dfs, graph_schema)

Nitpick: forgot to change these names..

@Mats-SX Mats-SX merged commit 2c069a5 into neo4j:main Dec 21, 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.

None yet

3 participants