[DOC] MyST examples and other visualization changes#195
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes several visualization-related improvements and refactors SegmentedSFT to support TRX files lazily (avoiding the eager to_sft() conversion that loaded all streamlines into memory). It also renames the user-facing bundle "V1V3" to "Early Visual" and exposes/extends a few visualization knobs (per-image trim_buffer, n_sls_viz for the fury backend).
Changes:
- Refactor
SegmentedSFTto handle TRX inputs lazily via anis_trxbranch; convertbundle_idxsfrom a dict attribute to a method, addto_rasmm/get_lengthshelpers, and update all internal callers. - Rename "Left/Right V1V3" → "Left/Right Early Visual" in the bundle dict and viz color map.
- Add a configurable
trim_buffertoadd_img/trim/bboxand exposen_sls_vizon the furyvisualize_bundlesAPI.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| AFQ/utils/streamlines.py | Adds TRX-aware SegmentedSFT path, converts bundle_idxs to a method, adds to_rasmm/get_lengths. |
| AFQ/tasks/segmentation.py | Updates callers to use get_lengths()/to_rasmm(); reaches into new private _bundle_idxs. |
| AFQ/api/group.py | Switches bundle_idxs[b] → bundle_idxs(b) and uses to_rasmm(). |
| AFQ/api/bundle_dict.py | Renames "V1V3" subbundles to "Early Visual". |
| AFQ/viz/utils.py | Renames V1V3 color entries, adds trim_buffer/bbox buffer, updates tract_generator for new API. |
| AFQ/viz/fury_backend.py | Exposes n_sls_viz parameter to visualize_bundles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
arokem
left a comment
There was a problem hiding this comment.
Integration of trx is excellent, but the added complexity would benefit from docs/testing.
|
No more gifs, huh? That format does have some advantages (e.g., more portable than MP4, I believe). |
|
For example, try posting an mp4 into a GitHub conversation. Does that work? |
|
It looks like GitHub only wants files less than 10MB currently, so I had to reduce the quality significantly. Luckily, I think it compressed well. mp4s will be much better for making things visually appealing and not stuttering. The GIF had that problem when I slowed it down, and it's just because GIFs barely compress, so we are always compromising frame rate / visual resolution / number of frames. This is an HBN subject test.mp4 |
|
This looks really good! |
|
I think that this is probably ready to merge? I re-triggered the documentation build, which failed previously. Hopefully these changes are not adding much in terms of memory pressure or runtime, and that failure was just a glitch. |
|
Yes, I think it is the mp4 generation that is causing problems right now, then we can merge |
|
@arokem I decided to ditch sphinx gallery and move all the examples to myst format. I think it simplified the code, especially around embedding videos, and it may solve our current issues with examples getting keyboard interrupts. What do you think? |
|
I think it's a great idea! But I couldn't find the examples by navigating the current documentation build. Did they get edited out of the table of contents? |
|
Yes, I just noticed the same thing. I think they are not being included, as well as the auto-generated kwargs and methods pages. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 68 out of 69 changed files in this pull request and generated 11 comments.
Comments suppressed due to low confidence (1)
docs/source/examples/tutorial_examples/plot_007_rois.md:200
- This display loop iterates over
enumerate(figs), butfigsis only defined inside the previous cell's loop and will contain only the last tract's figures. As a result, earlier tract PNGs won't be embedded (and the loop may reference the wrong set of indices). Use a filename glob per bundle instead of relying on the lastfigsvalue.
List of Changes: