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.

Sign up for GitHub

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

Jump to bottom

refactor: use graph-data-science-client arrow client instead of neo4j-arrow #7

Merged
merged 19 commits into from
Jan 10, 2025
Merged

refactor: use graph-data-science-client arrow client instead of neo4j-arrow #7

merged 19 commits into from
Jan 10, 2025

Conversation

dhrudevalia
Copy link
Contributor

Issue

neo4j-arrow use is being deprecated in favour of the client from graph-data-science-client maintained by the GDS team.

Fix

  • Replace all instances of neo4j-arrow.client with GDSArrowClient.
  • Copy the internal graph model (model.py) from neo4j-arrow
  • Copy the mapping functions (mapper.py) from neo4j-arrow
  • Fix typings
  • Update internal logic to match new function parameters and return types

@dhrudevalia dhrudevalia self-assigned this Jan 2, 2025
@dhrudevalia dhrudevalia marked this pull request as ready for review January 2, 2025 14:46
@dhrudevalia dhrudevalia requested a review from a team as a code owner January 2, 2025 14:46
venikkin
venikkin approved these changes Jan 2, 2025
View reviewed changes
Copy link

@venikkin venikkin left a comment

Choose a reason for hiding this comment

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

Not very familiar with Arrow or GDS client, so maybe it makes sense to seek an additional reviewer. But overall it code looks very good and clear. So as long it was tested, PR looks good to me. Added few nitpicks.

pyproject.toml Outdated Show resolved Hide resolved
src/templates/bigquery.py Outdated Show resolved Hide resolved
ali-ince
ali-ince reviewed Jan 3, 2025
View reviewed changes
Copy link
Contributor

@ali-ince ali-ince left a comment

Choose a reason for hiding this comment

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

Great work! Left a few comments and questions.

pyproject.toml Outdated Show resolved Hide resolved
src/templates/bigquery.py Outdated Show resolved Hide resolved
src/templates/bigquery.py Outdated Show resolved Hide resolved
src/templates/bigquery.py Outdated Show resolved Hide resolved
src/templates/bigquery.py Outdated Show resolved Hide resolved
@dhrudevalia dhrudevalia requested a review from ali-ince January 3, 2025 12:34
ali-ince
ali-ince approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@ali-ince ali-ince left a comment

Choose a reason for hiding this comment

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

Looks good!

@dhrudevalia dhrudevalia merged commit f9fb22e into main Jan 10, 2025
4 checks passed
@dhrudevalia dhrudevalia deleted the dd/migrate_to_gds_client branch January 10, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ali-ince ali-ince ali-ince approved these changes

@venikkin venikkin venikkin approved these changes

Assignees

@dhrudevalia dhrudevalia

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dhrudevalia @venikkin @ali-ince