Skip to content

WIP: Allow partially updating the track IDs#151

Closed
yfukai wants to merge 11 commits intoroyerlab:mainfrom
yfukai:fukai/partial_update_track_id
Closed

WIP: Allow partially updating the track IDs#151
yfukai wants to merge 11 commits intoroyerlab:mainfrom
yfukai:fukai/partial_update_track_id

Conversation

@yfukai
Copy link
Copy Markdown
Contributor

@yfukai yfukai commented Aug 22, 2025

This PR intends to allow partially updating the track IDs for GraphView while maintaining the overall consistency of the IDs. When users call the assign_track_id for GraphView, the track IDs for nodes outside the view are also updated to maintain the consistency.
This assumes that #150 is marged.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 22, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.11%. Comparing base (58cdd0d) to head (0f4f947).

Files with missing lines Patch % Lines
src/tracksdata/graph/_graph_view.py 87.50% 1 Missing and 1 partial ⚠️
src/tracksdata/graph/_sql_graph.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
+ Coverage   88.03%   88.11%   +0.07%     
==========================================
  Files          49       49              
  Lines        3243     3255      +12     
  Branches      544      545       +1     
==========================================
+ Hits         2855     2868      +13     
  Misses        231      231              
+ Partials      157      156       -1     

☔ 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.

Copy link
Copy Markdown
Member

@JoOkuma JoOkuma left a comment

Choose a reason for hiding this comment

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

Hey @yfukai,
I have seen this is a draft PR, so I'm just leaving some thoughts and not a full review.

I initially didn't implement assign_track_ids for the SQLGraph because it essentially needs the whole graph topology into memory, which is equivalent to loading it in memory as a view, which can be achieved through sql_graph.filter().subgraph().

Maybe this could be called internally by us to make SQLGraph API consistent.
filter().subgraph() copies all the content of the graph, we could have a new method for the SQLFilter that returns only the graph topology filter().rx_graph or similar.

Copy link
Copy Markdown
Member

@JoOkuma JoOkuma left a comment

Choose a reason for hiding this comment

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

@yfukai, I'm leaving tomorrow for vacation, so I'll only be able to review this after September 8th

@yfukai
Copy link
Copy Markdown
Contributor Author

yfukai commented Aug 22, 2025

@JoOkuma Hi, thanks for the comment! Yeah I was thinking something similar and I'll try to address in this PR. Enjoy your vacation!

@yfukai
Copy link
Copy Markdown
Contributor Author

yfukai commented Aug 23, 2025

Just note for myself: I started to work on with the use cases of partially updating the track IDs in a manual correction software (usually the updates are partial).

I was planning to use the

rx_graph.filter(node_id=node_ids).subgraph().assign_track_ids()

pattern where the node_ids are the updated node IDs by manual correction, but noticed that this involves copying the node props to the filtered subgraph. Maybe I should implement something like

rx_graph.assign_track_ids(node_id=node_ids)

which make a shallow "topology" copy of the graph and the assign the IDs. I'll check if there's an option to make the shallow copy in the former option and then decide.

@yfukai
Copy link
Copy Markdown
Contributor Author

yfukai commented Aug 23, 2025

Update: I noticed I could do

rx_graph.filter(node_id=node_ids).subgraph(node_attr_keys=[], edge_attr_keys=[]).assign_track_ids()

Maybe this is sufficient?

@yfukai
Copy link
Copy Markdown
Contributor Author

yfukai commented Aug 24, 2025

Note for myself: Digging into the error of the benchmarking.py, I noticed what I'm implementing actually does not match with the usecase of pruning the edges from the solution... I'll probably go with the

rx_graph.assign_track_ids(node_id=node_ids)

option.

@yfukai
Copy link
Copy Markdown
Contributor Author

yfukai commented Sep 2, 2025

I believe I messed up this PR. I'll open another one with copying a part of code from this one.

@yfukai yfukai closed this Sep 2, 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.

3 participants