Skip to content

gds/rel properties#173

Merged
FlorentinD merged 4 commits into
mainfrom
gds/rel-properties
Jun 6, 2025
Merged

gds/rel properties#173
FlorentinD merged 4 commits into
mainfrom
gds/rel-properties

Conversation

@FlorentinD

@FlorentinD FlorentinD commented May 28, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@adamnsch adamnsch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice! But I think we should still provide the opportunity to not fetch any properties. I would add a relationship_properties: Optional[list[str]] = None argument. None means we fetch everything, and [] means we fetch nothing

@adamnsch

Copy link
Copy Markdown
Collaborator

Nice! But I think we should still provide the opportunity to not fetch any properties. I would add a relationship_properties: Optional[list[str]] = None argument. None means we fetch everything, and [] means we fetch nothing

I suppose GDS only supports having one rel prop, so could even be a boolean 🤷

@FlorentinD

Copy link
Copy Markdown
Collaborator Author

@adamnsch the GDS graph can have multiple relationship types -- multiple rel properties.
Also some algorithms produce multiple properties. And at last, you can project multiple properties to a relationship.

@FlorentinD

Copy link
Copy Markdown
Collaborator Author

#173 (review)

for node-properties we default to no properties if additional_node_properties is not given.
Should we stick to the logic?
I was also confused now by the prefix additional. They are all from the projected graph.
I would have thought additional gets sources from the db and the others from GDS

@adamnsch

adamnsch commented Jun 2, 2025

Copy link
Copy Markdown
Collaborator

@adamnsch the GDS graph can have multiple relationship types -- multiple rel properties. Also some algorithms produce multiple properties. And at last, you can project multiple properties to a relationship.

I thought for sure it was only possible to have one relationship property per relationship type. But ok!

@adamnsch

adamnsch commented Jun 2, 2025

Copy link
Copy Markdown
Collaborator

#173 (review)

for node-properties we default to no properties if additional_node_properties is not given. Should we stick to the logic? I was also confused now by the prefix additional. They are all from the projected graph. I would have thought additional gets sources from the db and the others from GDS

I think "additional" refers to "in addition to the size_property that is already fetched". But yes, we might want to rethink the API now that we have the on-hover support. Maybe something we can discuss on Friday?

@adamnsch

adamnsch commented Jun 2, 2025

Copy link
Copy Markdown
Collaborator

Nice! But I think we should still provide the opportunity to not fetch any properties. I would add a relationship_properties: Optional[list[str]] = None argument. None means we fetch everything, and [] means we fetch nothing

I suppose GDS only supports having one rel prop, so could even be a boolean 🤷

Actually never mind this, let's just fetch everything

@FlorentinD FlorentinD merged commit 86fa0d6 into main Jun 6, 2025
11 checks passed
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