[ENH]: Add TemporalSlicer #443

2025-04-07T15:47:46Z

synchon commented

2025-04-07 15:47:46 +00:00

(Migrated from github.com)

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?

To have the option to define which segment of the time series to use for extracting connectomes (define start and end point). For example, cut a 20-minute sequence after 10 minutes and extract connectomes from only the second part.

Use-Case: We have a 24 minute sequence with a task and a control task performed sequentially and want to analyze them separately.

How do you imagine this integrated in junifer?

Cut the time series before calculating the connectomes.

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

Anything else to say?

@kaurao

### Are you requiring a new dataset or marker? - [x] I understand this is not a marker or dataset request ### Which feature do you want to include? To have the option to define which segment of the time series to use for extracting connectomes (define start and end point). For example, cut a 20-minute sequence after 10 minutes and extract connectomes from only the second part. Use-Case: We have a 24 minute sequence with a task and a control task performed sequentially and want to analyze them separately. ### How do you imagine this integrated in junifer? Cut the time series before calculating the connectomes. ### Do you have a sample code that implements this outside of junifer? ```shell ``` ### Anything else to say? @kaurao

synchon commented

2025-03-26 14:41:07 +00:00

(Migrated from github.com)

@jkroell Hi! Thanks for the feature request. Not sure I understand it correctly, do you want an option to perform temporal slicing at the marker level or do you want a preprocessor?

kaurao commented

2025-03-26 14:57:21 +00:00

(Migrated from github.com)

I think this can be implemented at different levels, either in preprocessing or in the marker. I guess making it part of preprocessing with make it more general and applicable across different markers?

synchon commented

2025-03-26 15:01:21 +00:00

(Migrated from github.com)

@kaurao Ok then we go with a new preprocessor.

fraimondo (Migrated from github.com) reviewed

2025-04-07 15:47:46 +00:00

codecov[bot] commented

2025-04-07 15:54:08 +00:00

(Migrated from github.com)

Attention: Patch coverage is 98.41270% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.09%. Comparing base (b9013e7) to head (57acd73).
Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
junifer/preprocess/_temporal_slicer.py	98.41%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #443      +/-   ##
==========================================
+ Coverage   86.13%   91.09%   +4.96%     
==========================================
  Files         133      134       +1     
  Lines        5617     5422     -195     
  Branches      950      903      -47     
==========================================
+ Hits         4838     4939     +101     
+ Misses        600      309     -291     
+ Partials      179      174       -5

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`
junifer	`91.09% <98.41%> (+4.96%)`	⬆️

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

Files with missing lines	Coverage Δ
junifer/pipeline/pipeline_component_registry.py	`94.64% <ø> (ø)`
junifer/preprocess/_temporal_slicer.py	`98.41% <98.41%> (ø)`

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/443?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report Attention: Patch coverage is `98.41270%` with `1 line` in your changes missing coverage. Please review. > Project coverage is 91.09%. Comparing base [(`b9013e7`)](https://app.codecov.io/gh/juaml/junifer/commit/b9013e7a306e7a56c0dc0483cc48ab53c9bb6787?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) to head [(`57acd73`)](https://app.codecov.io/gh/juaml/junifer/commit/57acd732ed0b269406fdedd2bc228abe15743c70?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml). > Report is 21 commits behind head on main. | [Files with missing lines](https://app.codecov.io/gh/juaml/junifer/pull/443?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Patch % | Lines | |---|---|---| | [junifer/preprocess/\_temporal\_slicer.py](https://app.codecov.io/gh/juaml/junifer/pull/443?src=pr&el=tree&filepath=junifer%2Fpreprocess%2F_temporal_slicer.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL190ZW1wb3JhbF9zbGljZXIucHk=) | 98.41% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/juaml/junifer/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | <details><summary>Additional details and impacted files</summary> [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/443/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://app.codecov.io/gh/juaml/junifer/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #443 +/- ## ========================================== + Coverage 86.13% 91.09% +4.96% ========================================== Files 133 134 +1 Lines 5617 5422 -195 Branches 950 903 -47 ========================================== + Hits 4838 4939 +101 + Misses 600 309 -291 + Partials 179 174 -5 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/443/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [docs](https://app.codecov.io/gh/juaml/junifer/pull/443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `100.00% <ø> (ø)` | | | [junifer](https://app.codecov.io/gh/juaml/junifer/pull/443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `91.09% <98.41%> (+4.96%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/juaml/junifer/pull/443?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/pipeline/pipeline\_component\_registry.py](https://app.codecov.io/gh/juaml/junifer/pull/443?src=pr&el=tree&filepath=junifer%2Fpipeline%2Fpipeline_component_registry.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9waXBlbGluZS9waXBlbGluZV9jb21wb25lbnRfcmVnaXN0cnkucHk=) | `94.64% <ø> (ø)` | | | [junifer/preprocess/\_temporal\_slicer.py](https://app.codecov.io/gh/juaml/junifer/pull/443?src=pr&el=tree&filepath=junifer%2Fpreprocess%2F_temporal_slicer.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL190ZW1wb3JhbF9zbGljZXIucHk=) | `98.41% <98.41%> (ø)` | | ... and [26 files with indirect coverage changes](https://app.codecov.io/gh/juaml/junifer/pull/443/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) </details> <details><summary> :rocket: New features to boost your workflow: </summary> - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. </details>

synchon commented

2025-04-07 15:55:54 +00:00

(Migrated from github.com)

@jkroell How do you like this interface?

preprocess:
  - kind: TemporalSlicer
    start: 0 # int, in second
    stop: 168 # int, in second
    t_r: 2.0  # float, in second (TR)

@jkroell How do you like this interface? ```yaml preprocess: - kind: TemporalSlicer start: 0 # int, in second stop: 168 # int, in second t_r: 2.0 # float, in second (TR) ```

fraimondo commented

2025-04-07 16:00:31 +00:00

(Migrated from github.com)

@synchon @kaurao @jkroell Does it make sense to add the positibility to specify intervals in several ways?

[tstart, tend]
[tstart, tstart+len]

Ej:

10 minutes starting at 0: tstart=0, len=10*60
from minute 10 to minute 20: tstart=10*60, tend=20*60

@synchon @kaurao @jkroell Does it make sense to add the positibility to specify intervals in several ways? * `[tstart, tend]` * `[tstart, tstart+len]` Ej: * 10 minutes starting at 0: `tstart=0, len=10*60` * from minute 10 to minute 20: `tstart=10*60, tend=20*60`

fraimondo commented

2025-04-07 16:04:04 +00:00

(Migrated from github.com)

@jkroell How do you like this interface?

preprocess:
  - kind: TemporalSlicer
    start: 0 # int, in second
    stop: 168 # int, in second
    t_r: 2.0  # float, in second (TR)

start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors.

> @jkroell How do you like this interface? > > ```yaml > preprocess: > - kind: TemporalSlicer > start: 0 # int, in second > stop: 168 # int, in second > t_r: 2.0 # float, in second (TR) > ``` start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors.

synchon commented

2025-04-07 16:04:08 +00:00

(Migrated from github.com)

Adding a len doesn't make sense to me as you can calculate that and be sure of your tend depending on t_r. But maybe, as an end-user, Jean's thoughts might differ.

Adding a `len` doesn't make sense to me as you can calculate that and be sure of your `tend` depending on `t_r`. But maybe, as an end-user, Jean's thoughts might differ.

fraimondo commented

2025-04-07 16:05:42 +00:00

(Migrated from github.com)

Adding a len doesn't make sense to me as you can calculate that and be sure of your tend depending on t_r. But maybe, as an end-user, Jean's thoughts might differ.

Why can't we support both?

It's easier to copy/paste a yaml and then change the tstart if you want to analyse 10 minutes from T0 and 10 minutes from T1, where T1 is a number that we know from the acquisition.

> Adding a `len` doesn't make sense to me as you can calculate that and be sure of your `tend` depending on `t_r`. But maybe, as an end-user, Jean's thoughts might differ. Why can't we support both? It's easier to copy/paste a yaml and then change the tstart if you want to analyse 10 minutes from T0 and 10 minutes from T1, where T1 is a number that we know from the acquisition.

synchon commented

2025-04-07 16:06:09 +00:00

(Migrated from github.com)

@jkroell How do you like this interface?
preprocess:
  - kind: TemporalSlicer
    start: 0 # int, in second
    stop: 168 # int, in second
    t_r: 2.0  # float, in second (TR)
start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors.

Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError.

> > @jkroell How do you like this interface? > > ```yaml > > preprocess: > > - kind: TemporalSlicer > > start: 0 # int, in second > > stop: 168 # int, in second > > t_r: 2.0 # float, in second (TR) > > ``` > > start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors. Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError.

synchon commented

2025-04-07 16:07:31 +00:00

(Migrated from github.com)

Adding a len doesn't make sense to me as you can calculate that and be sure of your tend depending on t_r. But maybe, as an end-user, Jean's thoughts might differ.

Why can't we support both?

It's easier to copy/paste a yaml and then change the tstart if you want to analyse 10 minutes from T0 and 10 minutes from T1, where T1 is a number that we know from the acquisition.

Of course we can, not a problem. Just wanted to make sure there's a proper use case.

> > Adding a `len` doesn't make sense to me as you can calculate that and be sure of your `tend` depending on `t_r`. But maybe, as an end-user, Jean's thoughts might differ. > > Why can't we support both? > > It's easier to copy/paste a yaml and then change the tstart if you want to analyse 10 minutes from T0 and 10 minutes from T1, where T1 is a number that we know from the acquisition. Of course we can, not a problem. Just wanted to make sure there's a proper use case.

synchon commented

2025-04-07 16:11:37 +00:00

(Migrated from github.com)

@jkroell How do you like this interface?
preprocess:
  - kind: TemporalSlicer
    start: 0 # int, in second
    stop: 168 # int, in second
    t_r: 2.0  # float, in second (TR)
start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors.
Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError.

This is taken care of now.

> > > @jkroell How do you like this interface? > > > ```yaml > > > preprocess: > > > - kind: TemporalSlicer > > > start: 0 # int, in second > > > stop: 168 # int, in second > > > t_r: 2.0 # float, in second (TR) > > > ``` > > > > > > start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors. > > Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError. This is taken care of now.

fraimondo commented

2025-04-07 16:11:40 +00:00

(Migrated from github.com)

Adding a len doesn't make sense to me as you can calculate that and be sure of your tend depending on t_r. But maybe, as an end-user, Jean's thoughts might differ.

Why can't we support both?
It's easier to copy/paste a yaml and then change the tstart if you want to analyse 10 minutes from T0 and 10 minutes from T1, where T1 is a number that we know from the acquisition.

Of course we can, not a problem. Just wanted to make sure there's a proper use case.

If we can do the math for the user, better.

I had this use case recently because someone merged resting with a task so I had to discard some volumes at the begining. Which also makes me think that in that case I needed:

tend=None and len=None should mean until the end of the recording.

Another possible use case:

tend<0 could also mean stop end tend seconds from the end. Basically, crop tend seconds from the end.

> > > Adding a `len` doesn't make sense to me as you can calculate that and be sure of your `tend` depending on `t_r`. But maybe, as an end-user, Jean's thoughts might differ. > > > > > > Why can't we support both? > > It's easier to copy/paste a yaml and then change the tstart if you want to analyse 10 minutes from T0 and 10 minutes from T1, where T1 is a number that we know from the acquisition. > > Of course we can, not a problem. Just wanted to make sure there's a proper use case. If we can do the math for the user, better. I had this use case recently because someone merged resting with a task so I had to discard some volumes at the begining. Which also makes me think that in that case I needed: * `tend=None` and `len=None` should mean until the end of the recording. Another possible use case: * `tend<0` could also mean stop _end `tend` seconds from the end_. Basically, crop `tend` seconds from the end.

github-actions[bot] commented

2025-04-07 16:30:02 +00:00

(Migrated from github.com)

PR Preview Action v1.6.0
🚀 View preview at https://juaml.github.io/junifer/pr-preview/pr-443/
Built to branch `gh-pages` at 2025-04-08 14:49 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.6.0 :---: | <p></p> :rocket: View preview at <br> https://juaml.github.io/junifer/pr-preview/pr-443/ <br><br> | <h6>Built to branch [`gh-pages`](https://github.com/juaml/junifer/tree/gh-pages) at 2025-04-08 14:49 UTC. <br> Preview will be ready when the [GitHub Pages deployment](https://github.com/juaml/junifer/deployments) is complete. <br><br> </h6>

jkroell commented

2025-04-08 09:16:07 +00:00

(Migrated from github.com)

@jkroell How do you like this interface?
preprocess:
  - kind: TemporalSlicer
    start: 0 # int, in second
    stop: 168 # int, in second
    t_r: 2.0  # float, in second (TR)
start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors.
Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError.
This is taken care of now.

I like the interface! Would it be an option to automatically get the TR, or do you think the user should set that himself?

> > > > @jkroell How do you like this interface? > > > > ```yaml > > > > preprocess: > > > > - kind: TemporalSlicer > > > > start: 0 # int, in second > > > > stop: 168 # int, in second > > > > t_r: 2.0 # float, in second (TR) > > > > ``` > > > > > > > > > start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors. > > > > > > Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError. > > This is taken care of now. I like the interface! Would it be an option to automatically get the TR, or do you think the user should set that himself?

fraimondo commented

2025-04-08 09:36:55 +00:00

(Migrated from github.com)

@jkroell How do you like this interface?
preprocess:
  - kind: TemporalSlicer
    start: 0 # int, in second
    stop: 168 # int, in second
    t_r: 2.0  # float, in second (TR)
start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors.
Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError.
This is taken care of now.
I like the interface! Would it be an option to automatically get the TR, or do you think the user should set that himself?

If the TR is not specified, it will be taken from the images. However, keep in mind that some software do not take care of the TR properly, so it's always good to have an option to fix it manually.

> > > > > @jkroell How do you like this interface? > > > > > ```yaml > > > > > preprocess: > > > > > - kind: TemporalSlicer > > > > > start: 0 # int, in second > > > > > stop: 168 # int, in second > > > > > t_r: 2.0 # float, in second (TR) > > > > > ``` > > > > > > > > > > > > start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors. > > > > > > > > > Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError. > > > > > > This is taken care of now. > > I like the interface! Would it be an option to automatically get the TR, or do you think the user should set that himself? If the TR is not specified, it will be taken from the images. However, keep in mind that some software do not take care of the TR properly, so it's always good to have an option to fix it manually.

jkroell commented

2025-04-08 09:45:28 +00:00

(Migrated from github.com)

@jkroell How do you like this interface?
preprocess:
  - kind: TemporalSlicer
    start: 0 # int, in second
    stop: 168 # int, in second
    t_r: 2.0  # float, in second (TR)
start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors.
Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError.
This is taken care of now.
I like the interface! Would it be an option to automatically get the TR, or do you think the user should set that himself?
If the TR is not specified, it will be taken from the images. However, keep in mind that some software do not take care of the TR properly, so it's always good to have an option to fix it manually.

Okay, makes sense! Looks good to me, then.

> > > > > > @jkroell How do you like this interface? > > > > > > ```yaml > > > > > > preprocess: > > > > > > - kind: TemporalSlicer > > > > > > start: 0 # int, in second > > > > > > stop: 168 # int, in second > > > > > > t_r: 2.0 # float, in second (TR) > > > > > > ``` > > > > > > > > > > > > > > > start and stop, if in seconds, should be floats. If we have sub-second TR we are in trouble. We'll most probably have rounding errors. > > > > > > > > > > > > Making them floats is a good idea to be more precise with slicing. I anyway do an int conversion to take care of that, most you get is an IndexError. > > > > > > > > > This is taken care of now. > > > > > > I like the interface! Would it be an option to automatically get the TR, or do you think the user should set that himself? > > If the TR is not specified, it will be taken from the images. However, keep in mind that some software do not take care of the TR properly, so it's always good to have an option to fix it manually. Okay, makes sense! Looks good to me, then.

👍 1

synchon commented

2025-04-08 10:13:22 +00:00

(Migrated from github.com)

@jkroell Was about to reply, but Fede already gave a nice answer.
@fraimondo Have added support for negative indexing in stop.

@jkroell Was about to reply, but Fede already gave a nice answer. @fraimondo Have added support for negative indexing in `stop`.

synchon commented

2025-04-08 14:17:04 +00:00

(Migrated from github.com)

@fraimondo Ok so now we can have stop=None which would take you to end and duration which is added to start to address the situations you described. I believe it's better to adjust the confounds as well?

The interface now looks like:

negative stop:

preprocess:
  - kind: TemporalSlicer
    start: 0.0 # float, in second
    stop: -2.0 # -ve float, in second

no end slicing:

preprocess:
  - kind: TemporalSlicer
    start: 10.0 # float, in second
    stop: null

duration:

preprocess:
  - kind: TemporalSlicer
    start: 10.0 # float, in second; start at 10s
    stop: null
    duration: 20.0 # float, in second; stop at 30s

Does it make sense now? @fraimondo @jkroell @kaurao

@fraimondo Ok so now we can have `stop=None` which would take you to end and `duration` which is added to `start` to address the situations you described. I believe it's better to adjust the confounds as well? The interface now looks like: - negative `stop`: ```yaml preprocess: - kind: TemporalSlicer start: 0.0 # float, in second stop: -2.0 # -ve float, in second ``` - no end slicing: ```yaml preprocess: - kind: TemporalSlicer start: 10.0 # float, in second stop: null ``` - duration: ```yaml preprocess: - kind: TemporalSlicer start: 10.0 # float, in second; start at 10s stop: null duration: 20.0 # float, in second; stop at 30s ``` Does it make sense now? @fraimondo @jkroell @kaurao

fraimondo commented

2025-04-09 18:13:57 +00:00

(Migrated from github.com)

@fraimondo Ok so now we can have stop=None which would take you to end and duration which is added to start to address the situations you described. I believe it's better to adjust the confounds as well?

The interface now looks like:

negative stop:
preprocess:
  - kind: TemporalSlicer
    start: 0.0 # float, in second
    stop: -2.0 # -ve float, in second
no end slicing:
preprocess:
  - kind: TemporalSlicer
    start: 10.0 # float, in second
    stop: null
duration:
preprocess:
  - kind: TemporalSlicer
    start: 10.0 # float, in second; start at 10s
    stop: null
    duration: 20.0 # float, in second; stop at 30s
Does it make sense now? @fraimondo @jkroell @kaurao

Yes! Perfect sense.

And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal.

> @fraimondo Ok so now we can have `stop=None` which would take you to end and `duration` which is added to `start` to address the situations you described. I believe it's better to adjust the confounds as well? > > The interface now looks like: > > * negative `stop`: > > ```yaml > preprocess: > - kind: TemporalSlicer > start: 0.0 # float, in second > stop: -2.0 # -ve float, in second > ``` > > * no end slicing: > > ```yaml > preprocess: > - kind: TemporalSlicer > start: 10.0 # float, in second > stop: null > ``` > > * duration: > > ```yaml > preprocess: > - kind: TemporalSlicer > start: 10.0 # float, in second; start at 10s > stop: null > duration: 20.0 # float, in second; stop at 30s > ``` > > Does it make sense now? @fraimondo @jkroell @kaurao Yes! Perfect sense. And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal.

👍 1

synchon commented

2025-04-10 08:23:14 +00:00

(Migrated from github.com)

And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal.

Ideally it should be after confound removal, but is there a case where one can do it before confound removal or can use the confounds in markers?

> And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal. Ideally it should be after confound removal, but is there a case where one can do it before confound removal or can use the confounds in markers?

fraimondo commented

2025-04-10 08:30:36 +00:00

(Migrated from github.com)

And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal.

Ideally it should be after confound removal, but is there a case where one can do it before confound removal or can use the confounds in markers?

We need to be consistent. So I agree with you. Any picking on the BOLD timeseries should also pick any other data that matches.

> > And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal. > > Ideally it should be after confound removal, but is there a case where one can do it before confound removal or can use the confounds in markers? We need to be consistent. So I agree with you. Any picking on the BOLD timeseries should also pick any other data that matches.

synchon commented

2025-04-10 08:31:18 +00:00

(Migrated from github.com)

And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal.

Ideally it should be after confound removal, but is there a case where one can do it before confound removal or can use the confounds in markers?

We need to be consistent. So I agree with you. Any picking on the BOLD timeseries should also pick any other data that matches.

Okay then I'll make the necessary changes.

> > > And you are right, we need to adjust the confounds so they match. Though I believe this should be used after confound removal. > > > > > > Ideally it should be after confound removal, but is there a case where one can do it before confound removal or can use the confounds in markers? > > We need to be consistent. So I agree with you. Any picking on the BOLD timeseries should also pick any other data that matches. Okay then I'll make the necessary changes.

synchon commented

2025-04-11 11:08:08 +00:00

(Migrated from github.com)

@fraimondo Updated the code to reflect confounds manipulation, I'll request a review, feel free to add more comments before reviewing.

fraimondo (Migrated from github.com) requested changes

2025-04-11 11:30:04 +00:00

junifer/preprocess/_temporal_slicer.py

					
				@ -0,0 +177,4 @@

				        # Convert slice range from seconds to indices

				        index = slice(int(self.start // t_r), int(stop // t_r))

Can we thorugh out of bounds exceptions ourselves?

Maybe also an INFO log to show the user what's being "sliced".

Can we thorugh out of bounds exceptions ourselves? Maybe also an INFO log to show the user what's being "sliced".

synchon (Migrated from github.com) reviewed

2025-04-14 09:26:48 +00:00

junifer/preprocess/_temporal_slicer.py

					
				@ -0,0 +177,4 @@

				        # Convert slice range from seconds to indices

				        index = slice(int(self.start // t_r), int(stop // t_r))