[ENH]: Raise/warn if element is not ran #319
No reviewers
Labels
No labels
CRITICAL
Stale
WIP
bug
concept
coordinate
dataset
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
maintenance
maps
marker
mask
on hold
parcellation
preprocess
question
ready
storage
template-space
triage
wontfix
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
juaml/junifer!319
Loading…
Reference in a new issue
No description provided.
Delete branch "enh/raise-invalid-element-selector"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Are you requiring a new dataset or marker?
Which feature do you want to include?
I just did this:
and then tried to run junifer with
--element sub-0001.The issue (which take me a lot of time to solve) is that
sub-0001is not in the elements list, since the replacement is without thesub-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.
github.com/juaml/junifer@a9e799caa3/junifer/api/functions.py (L170-L178)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 Do you need it for v0.0.4 or can we move it to v0.0.5?
It's not that easy to implement, mainly because
elementcan 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 computeprocessed_elementsjust by counting how many times we fit themcinside that for loop (Line 171)Yes that is a bit tricky.
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.
Codecov Report
Attention: Patch coverage is
0%with29 linesin your changes missing coverage. Please review.Additional details and impacted files
100.00% <ø> (ø)0.00% <0.00%> (ø)Flags with carried forward coverage won't be shown. Click here to find out more.
0.00% <0.00%> (ø)0.00% <0.00%> (ø)0.00% <0.00%> (ø)0.00% <0.00%> (ø)0.00% <0.00%> (ø)0.00% <0.00%> (ø)0.00% <0.00%> (ø)0.00% <0.00%> (ø)@ -243,7 +263,7 @@ def queue(kind: str,jobname: str = "junifer_job",This logic won't work in case element is part of a tuple of elements.
The idea is that we can have subject/session/task/etc as the element but only pass the "subject" part an the filter function will return all of the elements for that subject.
I think that the logic should be to check if all items in the
elementarray yielded at least one valid element.@ -243,7 +263,7 @@ def queue(kind: str,jobname: str = "junifer_job",Let me try to make it concrete and maybe it'll be easier to understand:
For this DataGrabber:
junifer list-elements <yaml>will give you:Now if I do:
will run all "four" elements of the "subject".
Now, if I instead do:
it'd just "exit" without the changes in this PR, but give you this:
with the current PR. Also, doing this:
would give you:
Is this what you meant or something else?
@ -243,7 +263,7 @@ def queue(kind: str,jobname: str = "junifer_job",That's what I meant. I'm kind of rusty now, but I had the impression that the PR would not account for that logic.
@ -243,7 +263,7 @@ def queue(kind: str,jobname: str = "junifer_job",Resolving it then.
Can you add a test case for the case we discussed?
Ej, sub-01 exists, but ses-02 does not.
Added now.
Once CI is green, LGTM!