[BUG]: Correct path propagation logic for ReHoEstimator and ALFFEstimator

[synchon]

• description of feature/fix
• tests added/passed
• add an entry for the latest changes

This PR fixes the path propagation logic for ReHoEstimator and ALFFEstimator based on the input data space. If the input data space is "native", the original input data path is passed down for further use in get_coordinates() (as of now) for transforming coordinates to subject-"native" template space, else the actual ReHo and (f)ALFF map paths are passed down.

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (dec59fd) 89.53% compared to head (210d83a) 89.45%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #286      +/-   ##
==========================================
- Coverage   89.53%   89.45%   -0.09%     
==========================================
  Files          98       98              
  Lines        4404     4408       +4     
  Branches      847      849       +2     
==========================================
  Hits         3943     3943              
- Misses        321      323       +2     
- Partials      140      142       +2     

Flag	Coverage Δ
docs	100.00% <ø> (ø)
junifer	89.44% <75.00%> (-0.09%)	⬇️

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

Files	Coverage Δ
junifer/markers/falff/falff_base.py	76.19% <ø> (ø)
junifer/markers/reho/reho_base.py	90.00% <ø> (ø)
junifer/markers/reho/reho_parcels.py	92.59% <ø> (ø)
junifer/markers/reho/reho_spheres.py	93.10% <ø> (ø)
junifer/markers/falff/falff_estimator.py	93.93% <75.00%> (-1.94%)	⬇️
junifer/markers/reho/reho_estimator.py	68.18% <75.00%> (-0.90%)	⬇️

[github-actions]

PR Preview Action v1.4.6
Preview removed because the pull request was closed.
2023-12-22 12:31 UTC

[fraimondo]

I don't fully agree with this solution though. I still do believe that the data and the path should point to the same object.

Nevertheless, this is a patch for the current main branch.

I would like to see this fixed when we have multi-mni space support. In that case, the img2imgcoord could simply get the T1w of the right space from templateflow and then we will not need this patches.

Can you please create an issue so we rollback this hack in the future?

[synchon]

I don't fully agree with this solution though. I still do believe that the data and the path should point to the same object.

Nevertheless, this is a patch for the current main branch.

I would like to see this fixed when we have multi-mni space support. In that case, the img2imgcoord could simply get the T1w of the right space from templateflow and then we will not need this patches.

Can you please create an issue so we rollback this hack in the future?

I don't have a better solution atm, please do share if you have a better one. I agree with you that the data and the path should point to the same object. The next thing we want is multi-MNI template space support, so if the solution is not too convincing, we can close this PR and get the multi-MNI support which would fix this as well. Of course, if you think we go with this, I'll make an issue to revert this.

[fraimondo]

For the moment this works and it's ok. I just want to document that this needs to be removed when multi-MNI space is added.