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: edge_index.dtype is float #31

Merged
merged 4 commits into from
Feb 9, 2024
Merged

fix: edge_index.dtype is float #31

merged 4 commits into from
Feb 9, 2024

Conversation

aMahanna
Copy link
Member

@aMahanna aMahanna commented Feb 8, 2024

The __process_adb_edge_df method is responsible for creating & appending to an EdgeStorage's edge_index.

Part of this code is responsible for mapping the ArangoDB Edges _from and _to values to the ArangoDB Vertex Collection Map (adb_map) to preserve the zero-based index value associated to the _from and _to IDs.

For a reason (not yet know), this mapping is returning a Pandas Series of floats instead of integers. These values are then piped into the edge_index property, resulting in edge_index.dtype being float...

This PR aims to address this issue by invoking astype(int) after invoking the Pandas map() function.

  • However, invoking astype(int) on a Pandas Series with possible NaN values will raise, so we must also introduce fillna(-1). This in turn affects the handling of Invalid Edges, hence why that code was also modified

(tests should fail)
@aMahanna
Copy link
Member Author

aMahanna commented Feb 8, 2024

notice how just adding the assertions from 2980240 will fail the CI

adbpyg_adapter/adapter.py Outdated Show resolved Hide resolved
Copy link
Contributor

@geenen124 geenen124 left a comment

Choose a reason for hiding this comment

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

lgtm

@aMahanna aMahanna merged commit d78a568 into master Feb 9, 2024
7 checks passed
@aMahanna aMahanna deleted the fix-edge-index-type branch February 9, 2024 14:18
aMahanna added a commit that referenced this pull request Feb 9, 2024
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.

2 participants