-
Notifications
You must be signed in to change notification settings - Fork 5
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
Automatically name index #1974
Automatically name index #1974
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,9 +16,12 @@ class Config: | |
add_missing_columns = True | ||
coerce = True | ||
|
||
@classmethod | ||
def _index_name(self) -> str: | ||
return "fid" | ||
@pa.dataframe_parser | ||
def _name_index(cls, df): | ||
# Node and Edge have different index names, avoid running both parsers | ||
if cls.__name__ not in ("NodeSchema", "EdgeSchema"): | ||
df.index.name = "fid" | ||
return df | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was confused for a while since I expected the methods in the subclasses in edge.py and node.py to override this. But when debugging a test failure I saw that first the node parser was applied, naming it "node_id", and then this one, making it end up with the wrong name. Hence the check for those class names. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we could go with that as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 007ee67 |
||
|
||
@classmethod | ||
def migrate(cls, df: Any, schema_version: int) -> Any: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are no longer needed because the parser will rename it for us.