Skip to content

Adding method to remove and add nodes to bbox spatial filter#159

Merged
JoOkuma merged 17 commits intoroyerlab:mainfrom
JoOkuma:jookuma/spatial-filter-sync
Nov 17, 2025
Merged

Adding method to remove and add nodes to bbox spatial filter#159
JoOkuma merged 17 commits intoroyerlab:mainfrom
JoOkuma:jookuma/spatial-filter-sync

Conversation

@JoOkuma
Copy link
Copy Markdown
Member

@JoOkuma JoOkuma commented Sep 9, 2025

No description provided.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 9, 2025

Codecov Report

❌ Patch coverage is 91.95402% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.32%. Comparing base (2bf5cb1) to head (f3bc4d7).

Files with missing lines Patch % Lines
src/tracksdata/graph/filters/_spatial_filter.py 81.08% 3 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   88.27%   88.32%   +0.05%     
==========================================
  Files          51       52       +1     
  Lines        3692     3769      +77     
  Branches      641      656      +15     
==========================================
+ Hits         3259     3329      +70     
- Misses        260      263       +3     
- Partials      173      177       +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.

@TeunHuijben TeunHuijben self-requested a review September 10, 2025 17:19
Copy link
Copy Markdown
Contributor

@TeunHuijben TeunHuijben left a comment

Choose a reason for hiding this comment

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

LGTM!

Sorry for the mess with merging other branches and main into this one,

@JoOkuma
Copy link
Copy Markdown
Member Author

JoOkuma commented Sep 12, 2025

@TeunHuijben , just to double check, even though you approved the PR.
Should we wait to see if there's anything else to add here with your recent work?

@TeunHuijben
Copy link
Copy Markdown
Contributor

If it is blocking anything else, go ahead and merge it, because I haven't encountered anything so far. Otherwise, give me two days to properly check all functionalities :)

@JoOkuma
Copy link
Copy Markdown
Member Author

JoOkuma commented Sep 15, 2025

@TeunHuijben, I'll leave this open and take your time

@TeunHuijben
Copy link
Copy Markdown
Contributor

TeunHuijben commented Sep 19, 2025

@JoOkuma, two changes:

  • node_{removed/added} signals in GraphView emit on the graph itself, not the _root (as we discussed)
  • added BBoxSpatialFilter._note_tree creation in it's add_node method when the filter was initialized with an empty graph, including test. (this should have been a separate PR I realize now, sorry!)

@yfukai
Copy link
Copy Markdown
Contributor

yfukai commented Nov 13, 2025

Maybe we should revive this? I have a similar kind of need.

@JoOkuma
Copy link
Copy Markdown
Member Author

JoOkuma commented Nov 13, 2025

Hey @yfukai,

I merged with main.
Can you test if this works for you? And what else is needed?

@yfukai
Copy link
Copy Markdown
Contributor

yfukai commented Nov 13, 2025

This looks great to me thanks @JoOkuma and @TeunHuijben !

@JoOkuma
Copy link
Copy Markdown
Member Author

JoOkuma commented Nov 17, 2025

I'm going to merge this. We can always revisit the implementation and make adjustments for improvement.
Thank you, @TeunHuijben, for the help.

@JoOkuma JoOkuma merged commit 30ef761 into royerlab:main Nov 17, 2025
6 of 7 checks passed
@JoOkuma JoOkuma deleted the jookuma/spatial-filter-sync branch November 17, 2025 22:41
@yfukai yfukai mentioned this pull request Dec 1, 2025
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.

4 participants