vesoft-inc / nebula-python

Client API of Nebula Graph in Python
191 stars 75 forks source link

Enhance the implementation of `properties(tag)` in `Node` #300

Closed haoxins closed 8 months ago

haoxins commented 8 months ago

Hi,

When we call as_node() from VertexData, https://github.com/vesoft-inc/nebula-python/blob/master/nebula3/sclient/BaseResult.py#L80 we define a tags to contain tag, but the list length always be 1. This lead to the method properties(tag) in Node must provide the name of tag. This is unnecessary and different from Relationship, https://github.com/vesoft-inc/nebula-python/blob/master/nebula3/data/DataObject.py#L1519

BTW, it seems that https://github.com/vesoft-inc/nebula-python/blob/master/nebula3/data/DataObject.py#L1342 is not used any more?

So my question is that can we change the method properties(tag) in Node, and then the signature will same as Relationship.

wey-gu commented 8 months ago

Thanks @haoxins !

This lead to the method properties(tag) in Node must provide the name of tag.

In the graph client case, a node could be multi-tagged but there are no multi-typed edges, thus those wrappers came with the nature that the node's properties are tag-key-mapped.

I think this as_node() is trying to align the data object of both g-client and s-client.

What is your suggestion to enable the ease of getting properties of tag in Node object from s-client?

BTW, it seems that https://github.com/vesoft-inc/nebula-python/blob/master/nebula3/data/DataObject.py#L1342 is not used any more?

You are right, that's a dead function of the class as for now.

I guess it's due to we have to manipulate edge.type(when it's a negative value, meaning this is scanned from the dst vertex's part), so we need that to instantiate the relationship, and for the node case, passing the vertex for the init is enough.

haoxins commented 8 months ago

thanks for the explanation I raised a PR to make the tag param optional https://github.com/vesoft-inc/nebula-python/pull/302/files

wey-gu commented 8 months ago

thanks for the explanation I raised a PR to make the tag param optional https://github.com/vesoft-inc/nebula-python/pull/302/files

I loved your implementation 👍