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
[FIX] fix positions markers on images on sphere masker reports #4285
Conversation
👋 @Remi-Gau Thanks for creating a PR! Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft. Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.
For new features:
For bug fixes:
We will review it as quick as possible, feel free to ping us with questions if needed. |
for visual tests from nilearn import datasets
from nilearn.maskers import NiftiSpheresMasker
dmn_coords = [(0, -52, 18), (-46, -68, 32), (46, -68, 32), (1, 50, -5)]
# plot sphere on MNI152 was unaffected before
report = NiftiSpheresMasker(dmn_coords).fit().generate_report()
report.save_as_html("report1.html")
# plot sphere on fitted image is fixed
masker = NiftiSpheresMasker(
dmn_coords,
)
dataset = datasets.fetch_development_fmri(n_subjects=10)
func_filename = dataset.func[0]
time_series = masker.fit(func_filename)
report = masker.generate_report()
report.save_as_html('report2.html') |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4285 +/- ##
==========================================
+ Coverage 91.85% 91.98% +0.12%
==========================================
Files 144 144
Lines 16419 16422 +3
Branches 3434 3436 +2
==========================================
+ Hits 15082 15105 +23
+ Misses 792 772 -20
Partials 545 545
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The change makes sense to me. |
Will update changelog and merge if CI is still OK. |
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.
adding some semantic line breaks in the problematic examples reported in the issue to trigger its build in CI to check the problem is solved.
Changes proposed in this pull request: