Skip to content

let graphview read missing attributes from root - #340

Open
TeunHuijben wants to merge 2 commits into
royerlab:mainfrom
TeunHuijben:root-fallback
Open

TeunHuijben wants to merge 2 commits into
royerlab:mainfrom
TeunHuijben:root-fallback

Conversation

@TeunHuijben

Copy link
Copy Markdown
Contributor

Problem:

GraphViews can be built using only a subset of node attributes. subgraph(node_attr_keys=[...]) lets you build a view without expensive columns, but the view then refuses to read them. If we query the GraphView with an attribute that is missing, it returns a KeyError because that key does not exist.

Idea

Actually, that key does exist, and it does in the root graph. So it is possible to get that key for those nodes from the root

Solution:

This PR adds GraphView(root_fallback=True), plumbed through subgraph(). When set, reading a node attribute key the view doesn't hold is served from the root for exactly the view's nodes, instead of raising.

Off by default. It only ever applies to views built with an explicit node_attr_keys list, so opting in costs one kwarg at a call site that's already passing one — and the fetch hits the root's storage, which for a SQL root means a query per read. When it's off, the KeyError now points at the flag if the root does have the key.

Notes:

  • attr_keys=None stays lean; it never pulls in excluded columns.
  • Read path only. Writes to a non-local key already went straight to the root and still do (so this PR only makes it more consistent)

@TeunHuijben

Copy link
Copy Markdown
Contributor Author

@JoOkuma / @cmalinmayor, any conceptual thoughts on fetching root attributes when the view doesn't have them?

The reason for this feature is the case of enormous SQL graphs on disk. We don't want the heavy masks in the view because of memory constraints, so I exclude the Mask attribute from the view. But they still need to be fetchable for the segmentation. Fetching the root is slightly more expensive, but it makes it possible to have very large graphs in memory. That is why this option defaults to False.

@TeunHuijben

Copy link
Copy Markdown
Contributor Author

@JoOkuma, I pushed a new commit that handles nested views. You might want to keep that in mind on your fork. Thanks!

This branch has not been deployed

No deployments
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.

1 participant