Skip to content

IndexError when slicing last run#415

Merged
smoia merged 9 commits intophysiopy:masterfrom
sangfrois:slice
Dec 12, 2022
Merged

IndexError when slicing last run#415
smoia merged 9 commits intophysiopy:masterfrom
sangfrois:slice

Conversation

@sangfrois
Copy link
Copy Markdown
Member

@sangfrois sangfrois commented Nov 18, 2021

Issue

Workflow crashes when the last recorded run doesn't have enough signal at the end. This is caused by the padding added at the end of the run.

A simple fix for that error was to deal with the exception.

Like this :

try:
    run_end = int(np.where(phys_in.timeseries[0] > end_sec)[0][0] + padding)
except IndexError:
    run_end = phys_in.timeseries[0].shape[0]

The end of the run becomes the last index of the timeserie.

Proposed Changes

  • slice4phys.py now contains a try: ... except IndexError statement.

Change Type

  • bugfix (+0.0.1)
  • minor (+0.1.0)
  • major (+1.0.0)
  • refactoring (no version update)
  • test (no version update)
  • infrastructure (no version update)
  • documentation (no version update)
  • other

Checklist before review

  • I added everything I wanted to add to this PR.
  • [Code or tests only] I wrote/updated the necessary docstrings.
  • [Code or tests only] I ran and passed tests locally.
  • [Documentation only] I built the docs locally.
  • My contribution is harmonious with the rest of the code: I'm not introducing repetitions.
  • My code respects the adopted style, especially linting conventions.
  • The title of this PR is explanatory on its own, enough to be understood as part of a changelog.
  • I added or indicated the right labels.
  • I added information regarding the timeline of completion for this PR.
  • Please, comment on my PR while it's a draft and give me feedback on the development!

@sangfrois sangfrois requested a review from smoia November 18, 2021 18:38
@smoia smoia added the BugFIX This PR generally closes a `Bug` issue, and increments the patch version (0.0.+1) label Nov 22, 2021
Copy link
Copy Markdown
Member

@smoia smoia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (let's wait for the tests)!

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 22, 2021

Codecov Report

Merging #415 (9073c65) into master (72ffc15) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #415   +/-   ##
=======================================
  Coverage   94.42%   94.42%           
=======================================
  Files           8        8           
  Lines         879      879           
=======================================
  Hits          830      830           
  Misses         49       49           
Impacted Files Coverage Δ
phys2bids/slice4phys.py 92.10% <ø> (ø)

@smoia
Copy link
Copy Markdown
Member

smoia commented Nov 22, 2021

@sangfrois do you have a file that we can use to test this particular situation?

try:
run_end = int(np.where(phys_in.timeseries[0] > end_sec)[0][0] + padding)
except IndexError:
run_end = phys_in.timeseries[0].shape[0]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sangfrois , could you add a warning here, something on the line:
LGR.warning('Some message that says that the file does not have enough timepoints hence the shape was used instead')

@smoia
Copy link
Copy Markdown
Member

smoia commented Dec 12, 2022

Hey @sangfrois! Could you pull from master, resolve conflicts, and push?

@sangfrois
Copy link
Copy Markdown
Member Author

sangfrois commented Dec 12, 2022

everything should be good to merge!
Let me know if you need me to do anything more!

Copy link
Copy Markdown
Member

@smoia smoia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@smoia smoia merged commit 4a4ee74 into physiopy:master Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugFIX This PR generally closes a `Bug` issue, and increments the patch version (0.0.+1)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants