-
Notifications
You must be signed in to change notification settings - Fork 1
[WIP] tracksdata backend #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v2-dev
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v2-dev #126 +/- ##
=========================================
Coverage ? 92.28%
=========================================
Files ? 50
Lines ? 2721
Branches ? 0
=========================================
Hits ? 2511
Misses ? 210
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🎉 wohoo! 🎉 |
…eature, removed all nx, made tests easier to read
|
PASSING! ✅🥳 @cmalinmayor, feel free to review the code and ask any questions regarding the many many changes necessary to make the backend |
cmalinmayor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not even close to done but the annotators action update I mentioned probably applies to other actions so I submit these now.
cmalinmayor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still didn't go through the tests thoroughly but lots of code comments to be getting on with! :)
| """ | ||
| lineages = nx.weakly_connected_components(self.tracks.graph) | ||
| max_id, ids_to_nodes = self._assign_ids(lineages, self.lineage_key) | ||
| lineages_internal = rx.weakly_connected_components(self.tracks.graph.rx_graph) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole section probably should be refactored to work better with the trackdata API. The internal external IDs seems fragile - this is a place where if we used tracksdata with SQL vs with rustworkx backend it would be a different implementation, right? Also, the _assign_ids helper could be refactored to do the efficient bulk update and then used for both lineage and track ids. There might be a better way to get the tracklet IDs that would eliminate the need for graph copying too.
| self.actions.append( | ||
| UpdateNodeSeg(tracks, new_value, all_pixels, added=True) | ||
| ) | ||
| tracks.graph[new_value][tracks.features.tracklet_key] = current_track_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this should be handled by the tracksAnnotator, right? I'm less confident this time but I'm suspicious. I can look closer later if needed
| assert np.array_equal(mask1.mask, mask2.mask) | ||
|
|
||
|
|
||
| def compute_node_attrs_from_masks( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh I think this is what the update function in the Annotators should do.
replaced nx in conftest and worked on test regarding adding/deleting nodes/edges