-
Notifications
You must be signed in to change notification settings - Fork 70
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
fixup for TWA-HP (gh1043) #1055
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- adds forgotten changelog entry from add framework for TWA head pos as MF destination #1043
- reverts some crufty changes to
_03_maxfilter
- remove
run
from the API ofcompute_twa_head_pos
(hopefully makes it clearer that it operates across all runs)
runs_tasks = get_runs_tasks( | ||
config=cfg, subject=subject, session=session, which=("runs",) | ||
)[0] | ||
) | ||
run = next(filter(lambda run_task: run_task[1] == task, runs_tasks))[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously there was a possibility that runs_tasks[0][0]
might be a run that wasn't present for the current task
; this should correct that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes to this file just revert to what it was like before #1043; it's cruft because I initially implemented TWA-HP in this file but then later moved it to _02_headpos
so these changes were no longer needed.
@@ -313,7 +312,6 @@ def main(*, config: SimpleNamespace) -> None: | |||
subject=subject, | |||
session=session, | |||
task=config.task or None, # default task is "" | |||
run=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to pass in run
here as it's never used
argh, osf.io is having a bad day :( |
failure appears real (finally!) |
Before merging …
docs/source/dev.md.inc
)