Adding update_node signal and connected them to spatial filters by yfukai · Pull Request #266 · royerlab/tracksdata

yfukai · 2026-02-19T06:51:14Z

This PR adds update_node signal to the graphs, and connected them to (BBox)SpatialFilter so that the rtrees are correctly udpated.

codecov-commenter · 2026-02-19T06:55:38Z

Codecov Report

❌ Patch coverage is 90.47619% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.22%. Comparing base (e10d8c3) to head (6aa3bc6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/tracksdata/graph/filters/_spatial_filter.py	74.35%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #266      +/-   ##
==========================================
+ Coverage   88.18%   88.22%   +0.03%     
==========================================
  Files          56       56              
  Lines        4308     4398      +90     
  Branches      749      780      +31     
==========================================
+ Hits         3799     3880      +81     
- Misses        319      324       +5     
- Partials      190      194       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JoOkuma

Hey @yfukai,
Thanks for the PR.
I'm a bit worried it might make things slower, but it seems safe based on the benchmarks CI.

src/tracksdata/graph/filters/_spatial_filter.py

JoOkuma · 2026-02-19T23:09:13Z

src/tracksdata/graph/filters/_spatial_filter.py

-                )
-            else:
+            if self._graph.num_nodes() == 0:
                raise ValueError("Spatial filter is not initialized")


Maybe also improve the error message.

Thanks, I found that the num_nodes must not be zero, since we emit the events after adding the nodes. I simply deleted this if clause.

src/tracksdata/graph/_graph_view.py

src/tracksdata/graph/_rustworkx_graph.py

JoOkuma · 2026-02-19T23:18:17Z

src/tracksdata/graph/_rustworkx_graph.py

            node_ids = self.node_ids()

+        emit_node_updated = is_signal_on(self.node_updated)
+        old_attrs_by_id = {node_id: dict(self._graph[node_id]) for node_id in node_ids} if emit_node_updated else None


Same as the other comment. Can you make this optional depending on is_signal_on

src/tracksdata/graph/_sql_graph.py

JoOkuma · 2026-02-19T23:23:51Z

src/tracksdata/graph/_sql_graph.py

+        updated_node_ids = self.node_ids() if node_ids is None else list(node_ids)
+        if len(updated_node_ids) == 0:
+            return
+        attr_keys = self.node_attr_keys()
+        old_df = self.filter(node_ids=updated_node_ids).node_attrs(attr_keys=[DEFAULT_ATTR_KEYS.NODE_ID, *attr_keys])
+        old_attrs_by_id = {
+            row[DEFAULT_ATTR_KEYS.NODE_ID]: {key: row[key] for key in attr_keys} for row in old_df.rows(named=True)
+        }
+
        self._update_table(self.Node, node_ids, DEFAULT_ATTR_KEYS.NODE_ID, attrs)
+        new_df = self.filter(node_ids=updated_node_ids).node_attrs(attr_keys=[DEFAULT_ATTR_KEYS.NODE_ID, *attr_keys])
+        new_attrs_by_id = {
+            row[DEFAULT_ATTR_KEYS.NODE_ID]: {key: row[key] for key in attr_keys} for row in new_df.rows(named=True)
+        }


Can you only trigger this only if is_signal_on

That's definitely better, thanks!

Co-authored-by: Jordão Bragantini <jordao.bragantini@gmail.com>

yfukai

I think I removed all the unnecessary dict calls and moved heavy computation inside the is_signal_on blocks! Maybe we can make events for bulk updates in the future.

src/tracksdata/graph/_graph_view.py

src/tracksdata/graph/filters/_spatial_filter.py

yfukai · 2026-02-20T06:00:03Z

src/tracksdata/graph/filters/_spatial_filter.py

-                )
-            else:
+            if self._graph.num_nodes() == 0:
                raise ValueError("Spatial filter is not initialized")


Thanks, I found that the num_nodes must not be zero, since we emit the events after adding the nodes. I simply deleted this if clause.

src/tracksdata/graph/filters/_spatial_filter.py

yfukai · 2026-02-20T06:23:55Z

src/tracksdata/graph/_sql_graph.py

+        updated_node_ids = self.node_ids() if node_ids is None else list(node_ids)
+        if len(updated_node_ids) == 0:
+            return
+        attr_keys = self.node_attr_keys()
+        old_df = self.filter(node_ids=updated_node_ids).node_attrs(attr_keys=[DEFAULT_ATTR_KEYS.NODE_ID, *attr_keys])
+        old_attrs_by_id = {
+            row[DEFAULT_ATTR_KEYS.NODE_ID]: {key: row[key] for key in attr_keys} for row in old_df.rows(named=True)
+        }
+
        self._update_table(self.Node, node_ids, DEFAULT_ATTR_KEYS.NODE_ID, attrs)
+        new_df = self.filter(node_ids=updated_node_ids).node_attrs(attr_keys=[DEFAULT_ATTR_KEYS.NODE_ID, *attr_keys])
+        new_attrs_by_id = {
+            row[DEFAULT_ATTR_KEYS.NODE_ID]: {key: row[key] for key in attr_keys} for row in new_df.rows(named=True)
+        }


That's definitely better, thanks!

src/tracksdata/graph/filters/_spatial_filter.py

JoOkuma

Hey @yfukai, sorry for the delay. I've been quite busy.

It looks good to me, but I left a few minor comments regarding the type casting.

Some seem unnecessary, and I'm worried about additional overhead.
Don't hesitate to let me know if they are required.

src/tracksdata/graph/_rustworkx_graph.py

src/tracksdata/graph/_sql_graph.py

Co-authored-by: Jordão Bragantini <jordao.bragantini@gmail.com>

yfukai · 2026-02-27T01:26:42Z

Thanks for the comments, I completely agree. Reflected.

JoOkuma · 2026-02-27T04:38:04Z

Thanks @yfukai, merged, I'll take a look at the other PRs tomorrow

added update_node signal and connected them to spatial filters

c871ecc

yfukai marked this pull request as ready for review February 19, 2026 06:51

yfukai mentioned this pull request Feb 19, 2026

Notifying GraphArrayView of the graph change #263

Closed

yfukai mentioned this pull request Feb 19, 2026

Notifying GraphArrayView of updates #267

Open

yfukai added 2 commits February 19, 2026 17:21

fixed failing benchmarks

4424166

bm timeout longer

c09d934

JoOkuma requested changes Feb 19, 2026

View reviewed changes

yfukai and others added 4 commits February 20, 2026 14:31

Update src/tracksdata/graph/filters/_spatial_filter.py

3ebc8e3

Co-authored-by: Jordão Bragantini <jordao.bragantini@gmail.com>

fixed unnecesary calls of dict()

6f3b1da

lint

e54d607

moved heavy computation to is_signal_on blocks

a756293

yfukai commented Feb 20, 2026

View reviewed changes

JoOkuma reviewed Feb 26, 2026

View reviewed changes

src/tracksdata/graph/filters/_spatial_filter.py Show resolved Hide resolved

Apply suggestion from @JoOkuma

a0b12fd

JoOkuma requested changes Feb 26, 2026

View reviewed changes

src/tracksdata/graph/_rustworkx_graph.py Outdated Show resolved Hide resolved

src/tracksdata/graph/_sql_graph.py Outdated Show resolved Hide resolved

src/tracksdata/graph/_sql_graph.py Outdated Show resolved Hide resolved

yfukai and others added 3 commits February 27, 2026 10:19

Update src/tracksdata/graph/_sql_graph.py

ad59450

Co-authored-by: Jordão Bragantini <jordao.bragantini@gmail.com>

Update src/tracksdata/graph/_sql_graph.py

c8286e6

Co-authored-by: Jordão Bragantini <jordao.bragantini@gmail.com>

Update src/tracksdata/graph/_rustworkx_graph.py

6aa3bc6

Co-authored-by: Jordão Bragantini <jordao.bragantini@gmail.com>

JoOkuma approved these changes Feb 27, 2026

View reviewed changes

JoOkuma merged commit 55c858b into royerlab:main Feb 27, 2026
7 checks passed

Conversation

yfukai commented Feb 19, 2026

Uh oh!

codecov-commenter commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JoOkuma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JoOkuma Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

yfukai Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoOkuma Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoOkuma Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

yfukai Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

yfukai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yfukai Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yfukai Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoOkuma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yfukai commented Feb 27, 2026

Uh oh!

JoOkuma commented Feb 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Feb 19, 2026 •

edited

Loading