Skip to content

Graph metadata API#197

Merged
JoOkuma merged 11 commits intoroyerlab:mainfrom
JoOkuma:jookuma/graph-metadata
Nov 13, 2025
Merged

Graph metadata API#197
JoOkuma merged 11 commits intoroyerlab:mainfrom
JoOkuma:jookuma/graph-metadata

Conversation

@JoOkuma
Copy link
Copy Markdown
Member

@JoOkuma JoOkuma commented Nov 10, 2025

@yfukai, could you review this PR?

It should address item 2 of #192

I added the option of automatically querying the shape to export it to napari.
This was requested internally

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 10, 2025

Codecov Report

❌ Patch coverage is 82.53968% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.14%. Comparing base (75f28d6) to head (e386063).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/tracksdata/array/_graph_array.py 69.23% 3 Missing and 1 partial ⚠️
src/tracksdata/graph/_rustworkx_graph.py 78.57% 2 Missing and 1 partial ⚠️
src/tracksdata/graph/_graph_view.py 71.42% 2 Missing ⚠️
src/tracksdata/graph/_sql_graph.py 95.23% 1 Missing ⚠️
src/tracksdata/nodes/_regionprops.py 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #197      +/-   ##
==========================================
- Coverage   88.17%   88.14%   -0.04%     
==========================================
  Files          51       51              
  Lines        3612     3669      +57     
  Branches      625      630       +5     
==========================================
+ Hits         3185     3234      +49     
- Misses        256      262       +6     
- Partials      171      173       +2     

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

@yfukai yfukai left a comment

Choose a reason for hiding this comment

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

The changes looks good to me, thanks!

Minor comments:

  • Could we also implement def remove_metadata(self, key) ?
  • shape argument of GraphArrayView constructor and to_ctc can also be omitted. It would be great if RegionPropsNodes andfrom_ctc saves the shape attribute.

@JoOkuma
Copy link
Copy Markdown
Member Author

JoOkuma commented Nov 12, 2025

Hi @yfukai, great suggestions! I addressed your comments.

Copy link
Copy Markdown
Contributor

@yfukai yfukai left a comment

Choose a reason for hiding this comment

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

Everything looks nice to me! I left a comment on a lazy import. Maybe the reason for importing lazily can be explained by a comment.

node_ids: list[int] | None = None,
return_id_update: bool = False,
) -> rx.PyDiGraph | tuple[rx.PyDiGraph, pl.DataFrame]:
from tracksdata.functional._rx import _assign_tracklet_ids
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why was this changed to lazy import?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added a note, it's local to avoid circular import

@yfukai
Copy link
Copy Markdown
Contributor

yfukai commented Nov 12, 2025

Oh but the tests are failing...

@yfukai
Copy link
Copy Markdown
Contributor

yfukai commented Nov 13, 2025

As soon as the test runs I think this PR is good to go!

@JoOkuma
Copy link
Copy Markdown
Member Author

JoOkuma commented Nov 13, 2025

Thank @yfukai

@JoOkuma JoOkuma merged commit 2682fd2 into royerlab:main Nov 13, 2025
6 of 7 checks passed
@JoOkuma JoOkuma deleted the jookuma/graph-metadata branch November 13, 2025 18:54
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