-
Notifications
You must be signed in to change notification settings - Fork 100
Add Rica integration for interactive ICA component visualization #1288
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: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1288 +/- ##
==========================================
+ Coverage 89.97% 90.06% +0.09%
==========================================
Files 29 30 +1
Lines 4497 4631 +134
Branches 746 771 +25
==========================================
+ Hits 4046 4171 +125
- Misses 303 308 +5
- Partials 148 152 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Update: Rica v2.0.0 Compatibility and Test Coverage ImprovementsJust pushed commit Changes Made:
Files Modified:
All pre-commit hooks passed successfully. |
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.
Pull request overview
This PR adds integration with Rica, an interactive web-based visualization tool for exploring ICA components. When users enable the --rica-report flag, tedana downloads Rica from GitHub releases, caches it locally, copies it to the output directory, and generates a launcher script for easy visualization.
Changes:
- Added
tedana/rica.pymodule with functions for downloading, caching, and setting up Rica reports - Added
--rica-reportflag totedanaandica_reclassifyworkflows - Added comprehensive test coverage in
tedana/tests/test_rica.pywith 18 unit tests - Added extensive documentation in
docs/outputs.rstcovering usage, troubleshooting, and advanced features
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tedana/rica.py |
New module implementing Rica download, caching, validation, and launcher script generation with cross-platform support |
tedana/workflows/tedana.py |
Added --rica-report flag and integration to call Rica setup after report generation |
tedana/workflows/ica_reclassify.py |
Added --rica-report flag and integration to call Rica setup after report generation |
tedana/tests/test_rica.py |
Comprehensive test suite with 18 unit tests covering all Rica functionality |
docs/outputs.rst |
Added detailed documentation section covering Rica usage, setup, troubleshooting, and advanced configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
What about adding a button to the HTML report to open the Rica website? If possible, it would be amazing to automatically select the tedana output folder as well. |
I feel like if users click on the tedana report HTML files, they could just as easily click on (or run) the open_rica_report.py script, which automatically opens the folder in Rica. WE can discuss at our meeting on Thu. |
handwerkerd
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.
I'm starting to look at this and noticed two issues:
- Nearly identical messages are logged twice in a row. Not sure which location is better, but we probably want the one that includes
python. Also, if the goal is to have this run, append the output directory onto open_rica_report.py so the command could run from the location the script was called:
INFO rica:setup_rica_report:603 Rica report setup complete. Run 'open_rica_report.py' to visualize results.
INFO tedana:tedana_workflow:1277 Rica report ready. Run 'python open_rica_report.py' to visualize results.
- On my Mac, I got a crash with
python open_rica_report.pybecause of
open_rica_report.py", line 72
rel_path = str(f.relative_to(cwd)).replace("\", "/")
It works with replace("\\", but I'm not sure if the double \ might cause problems in other systems.
|
I'm waiting for #1318 to check the report with the new button looks good in the artifacts. |
|
Fixed both issues from @handwerkerd's review in commit 075e090:
|
|
I think the test failures are because the reclassify integration tests are comparing against the lists of expected file outputs and |
This commit adds the ability to generate Rica interactive reports directly from tedana output directories. New features: - Add tedana/rica.py module for Rica download and report generation - Add --rica-report flag to tedana and ica_reclassify workflows - Download Rica from GitHub releases and cache locally - Generate open_rica_report.py launcher script in output directory - Cross-platform support for Linux, macOS, and Windows When --rica-report is used: 1. Rica files are downloaded from ME-ICA/rica GitHub releases (cached) 2. Files are copied to output_dir/rica/ 3. A launcher script (open_rica_report.py) is created 4. Users can run the script to visualize ICA components in Rica 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Document the --rica-report flag and explain how to use Rica for interactive ICA component visualization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Use typing module imports (Optional, Union, Tuple, Dict) instead of Python 3.10+ syntax (str | None, str | Path) for type annotations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove --rica-path CLI flag for simpler user experience - Rica now auto-downloads from GitHub if not found locally - Keep TEDANA_RICA_PATH env var for developers using custom builds - Add priority order: env var > bundled > cached > download - Add troubleshooting section to documentation - Add tests for local Rica path validation Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove favicon.ico from RICA_FILES to support Rica v2.0.0 (file no longer provided in releases) - Expand test coverage from 66% to 91% (24 → 43 tests) - Add Windows platform support tests (LOCALAPPDATA handling) - Add comprehensive network function tests for get_latest_rica_version - Improve test isolation with better mocking of is_rica_cached function Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Fix Windows path backslash replacement in tedana/rica.py line 404
Changed replace("\\\\", "/") to replace("\\", "/") for proper single backslash matching
- Remove favicon.ico references from docs/outputs.rst
Removed from Rica Output Files table, local bundle files list, and wget/mv commands
Rica v2.0.0 no longer includes favicon.ico
Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove --rica-report CLI flag from tedana and ica_reclassify workflows - Always generate open_rica_report.py launcher script in output directory - Move Rica download logic from tedana runtime to launcher script - Launcher now handles: checking cache, downloading from GitHub, copying files - Add --force-download flag to launcher for manual updates - Update tests to reflect simplified setup_rica_report() behavior - Update documentation to reflect new flow This simplifies user experience (no flag to remember), keeps tedana fast (no network calls during analysis), and provides on-demand Rica download. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add "Open in Rica" button fixed to top-right corner of report - Button opens a modal explaining how to run Rica locally - Modal includes command: python open_rica_report.py - "Learn more" button links to Rica GitHub repository - Modal can be closed via click outside, Escape key, or X button Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove unused shutil import - Break long lines in launcher script template to comply with line length limit Co-Authored-By: Claude Opus 4.5 <[email protected]>
Since Rica launcher script is now always generated, add it to all expected output files for integration tests that run tedana_workflow. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Addresses two issues found by @handwerkerd: 1. Use as_posix() instead of manual backslash replacement to convert Windows paths to forward slashes. This avoids escaping issues that caused crashes on Mac. 2. Log the full path to open_rica_report.py so users can run the command from any directory. Co-Authored-By: Claude Opus 4.5 <[email protected]>
The reclassify integration tests were failing because open_rica_report.py was not listed in the expected output files. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Fixed! Added
|
|
I have added the feedback from yesterday's meeting. Let me know what you think. |
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.
The changes all look good.
I did notice one issue. The bugs in rica visualization that you fixed appeared in rica when I opened it with python open_rica_report.py. The same report rendered properly when I opened it with my local version of rica using npm install; npm start I'm fairly sure what happened is that, when the program checks to see if it should download rica, it saw it was already present and didn't re-download. You might want some way to see if there's a newer version of rica available & to then download that version.
Co-authored-by: Dan Handwerker <[email protected]>
|
Ah good catch! I had the latest on mine and didn't see this. Thanks Dan! Can you try now @handwerkerd ? |
The open_rica_report.py launcher now checks GitHub for the latest Rica version on each run and automatically downloads updates if a newer version is available. This ensures users get the latest bug fixes without needing to use --force-download. - Always query GitHub API for latest version before using cache - Compare cached version with latest and download if outdated - Fall back to cached version if network is unavailable - Respect TEDANA_RICA_PATH env var (no auto-update when set) Requested by: handwerkerd Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
Adds Rica integration for interactive ICA component visualization. Every tedana run now creates a launcher script that handles Rica setup on demand.
Key Changes
open_rica_report.pyin the output directoryHow It Works
open_rica_report.pyin your output directory (full path is logged at end of run)python open_rica_report.pyAlternatively, open the HTML report and click the "Open in Rica" button in the top-right corner for instructions.
Features
pathlib.as_posix()for reliable Windows/Mac/Linux supportTEDANA_RICA_PATHenvironment variable for developers to use local Rica builds--force-downloadflag to launcher for manual updatesUsage
Test Plan
Files Changed
tedana/rica.py- Simplified to generate self-contained launcher scripttedana/workflows/tedana.py- Always calls setup_rica_report()tedana/workflows/ica_reclassify.py- Always calls setup_rica_report()tedana/tests/test_rica.py- Updated tests for new behaviordocs/outputs.rst- Updated documentationtedana/reporting/data/html/report_body_template.html- Added Rica button with popupCloses #1283.