-
Notifications
You must be signed in to change notification settings - Fork 124
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
Implment get_as_torch_geometric
#1200
Conversation
252e441
to
d086ece
Compare
800e65a
to
a4b6c62
Compare
LIST = "LIST" | ||
NODE = "NODE" | ||
REL = "REL" | ||
NODE_ID = "NODE_ID" |
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.
Update NODE_ID once Ziyi's PR is in.
|
||
PRIMARY_KEY_SYMBOL = "(PRIMARY KEY)" | ||
LIST_SYMBOL = "[]" | ||
result_str = self.query_result.connection._connection.get_node_property_names( |
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.
This comment is beyond this PR, but let me document it here:
The parsing here makes me think that our cpp interface string Connection::getNodePropertyNames(const string& tableName)
is incorrect. Instead of returning a string
, it's better to return vector<string>
, each inside which is a property name. The caller (shell) shall be responsible to organize them in a printable way.
continue | ||
|
||
# Ignore primary key | ||
if node_property_names[prop_name]["is_primary_key"]: |
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.
Why not put primary_key prop_name also into the ignored properties?
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.
Ignored property is not read at all, but for primary key, it still needs to be read as we need to maintain the pos_to_primary_key_dict
. I have changed the comment to Read primary key but do not add it to the node properties
.
from enum import Enum | ||
|
||
|
||
class Type(Enum): |
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.
Is there a way that we don't need to maintain a separate mapping in Python?
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.
As discussed in #1189, pybind11 has a memory leakage bug when binding enum class. We can change this Enum
to binding after the bug is fixed.
This PR implements a function
get_as_torch_geometric()
which converts the query result into atorch_geometric.data
object.Principles:
NODE
andREL
s are converted. Selected properties, such asa.isWorker
is ignored.INT64
andBOOL
types. Other types are ignored. Multi-dimensional lists are supported as long as the dimension is consistent and there is no sub-list of different length.torch.Tensor
.