Skip to content
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

[ENH]: Add test for JuniferNiftiSpheresMasker against nilearn's version #136

Merged
merged 8 commits into from
Jun 18, 2024

Conversation

synchon
Copy link
Member

@synchon synchon commented Jun 12, 2024

Are you requiring a new dataset or marker?

  • I understand this is not a marker or dataset request

Which feature do you want to include?

We reimplemented (by copying) a JuniferNiftiSpheresMasker that has the same functionality of NiftiSpheresMasker but allows to change the aggregation function.

The tests are also a copy of Nilearn's one.

If agg_function = 'mean', then they should both behave equally. I would like to have a test so we can corroborate this. They should be numerically equal.

How do you imagine this integrated in junifer?

One more test in junifer.external.nilearn.tests

Do you have a sample code that implements this outside of junifer?

No response

Anything else to say?

No response

@fraimondo fraimondo added enhancement New feature or request triage New issues waiting to be reviewed labels Nov 23, 2022
@fraimondo fraimondo added this to the 0.0.1 (alpha 0) milestone Nov 23, 2022
@fraimondo fraimondo removed the triage New issues waiting to be reviewed label Nov 23, 2022
@fraimondo fraimondo removed their assignment Nov 23, 2022
@fraimondo
Copy link
Contributor Author

For the moment, let's fix 0.10.0 as the latest supported nilearn version.

Onces 0.10.1 is released, let's add the tests and version-specific patches.

@fraimondo fraimondo removed this from the 0.0.2 (alpha 1) milestone Mar 28, 2023
@fraimondo fraimondo added the on hold Issue waiting on other issue(s) label Mar 28, 2023
@synchon synchon added this to the 0.0.5 (alpha 4) milestone Apr 9, 2024
Copy link

codecov bot commented Jun 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (d501374) to head (e205953).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #136   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1           
Flag Coverage Δ
docs 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link

github-actions bot commented Jun 12, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-06-18 16:36 UTC

@synchon synchon removed the on hold Issue waiting on other issue(s) label Jun 12, 2024
@synchon synchon requested a review from fraimondo June 12, 2024 16:24
@synchon
Copy link
Member

synchon commented Jun 12, 2024

#347 needs to go in before this as that sets the proper nilearn version we need for this, #333 and #335

@synchon synchon merged commit 03c06a5 into main Jun 18, 2024
14 of 18 checks passed
@synchon synchon deleted the update/junifer-nifti-spheres-masker branch June 18, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants