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]: Raise/warn if element is not ran #319

Open
1 task done
fraimondo opened this issue Apr 4, 2024 · 3 comments
Open
1 task done

[ENH]: Raise/warn if element is not ran #319

fraimondo opened this issue Apr 4, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request triage New issues waiting to be reviewed

Comments

@fraimondo
Copy link
Contributor

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?

I just did this:

  patterns:
    VBM_GM: 
        pattern: derivatives/fmriprep/sub-{subject}/anat/sub-{subject}_space-MNI152NLin2009cAsym_label-GM_probseg.nii.gz
        space: MNI152NLin2009cAsym
  replacements: 
    - subject

and then tried to run junifer with --element sub-0001.

The issue (which take me a lot of time to solve) is that sub-0001 is not in the elements list, since the replacement is without the sub- prefix.

Now, there was no issue/warning from the run function. It cloned the dataset, did nothing and dropped the dataset.

How do you imagine this integrated in junifer?

We should implement a check in which if some of the elements in the list is not selected by the filter, there's a warning/error.

Exactly here is where we pass through the filter.

if elements is not None:
for t_element in datagrabber_object.filter(
elements # type: ignore
):
mc.fit(datagrabber_object[t_element])
else:
for t_element in datagrabber_object:
mc.fit(datagrabber_object[t_element])

We should somehow keep a list of "unselected" elements so we can raise the warning.

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 Apr 4, 2024
@synchon
Copy link
Member

synchon commented Apr 5, 2024

@fraimondo Do you need it for v0.0.4 or can we move it to v0.0.5?

@fraimondo
Copy link
Contributor Author

It's not that easy to implement, mainly because element can be a part of the full tuple, to iterate over a subject's sessions.

However, what we can do for v0.0.4 is to raise a warning if len(elements) > len(processed_elements). We compute processed_elements just by counting how many times we fit the mc inside that for loop (Line 171)

@synchon
Copy link
Member

synchon commented Apr 5, 2024

It's not that easy to implement, mainly because element can be a part of the full tuple, to iterate over a subject's sessions.

Yes that is a bit tricky.

However, what we can do for v0.0.4 is to raise a warning if len(elements) > len(processed_elements). We compute processed_elements just by counting how many times we fit the mc inside that for loop (Line 171)

Although the solution is good, I would like to have a comprehensive solution as it's not a critical feature issue, but more of a user notification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage New issues waiting to be reviewed
Projects
None yet
Development

No branches or pull requests

2 participants