edickie
commented
2 years ago I think the major error here is that repetition time should never be hard coded.
We want it as an extra argument - or read from the file meta data.
I think I do this already in other scripts (cifti_clean?) so maybe we can just add that in?
On Tue, Nov 8, 2022 at 6:18 PM Dawn E. Smith @.***> wrote:
It seems like the main fix happening is the use of repetition time as the second argument to np.fft.fftfreq on line 178, instead of 1. The two tests in tests/functional/test_ciftify_falff.py pass both before and after the changes. Unfortunately, I'm not sure precisely how to update them to capture what these changes are fixing.
You can view, comment on, or merge this pull request online at:
https://github.com/edickie/ciftify/pull/177 Commit Summary
- ca78177 https://github.com/edickie/ciftify/pull/177/commits/ca7817761d77fc87b3fa3a2acb018934656c7efe [FIX] Add Soroush's fixes to ciftify_falff
- 566d63e https://github.com/edickie/ciftify/pull/177/commits/566d63e922ef056136196100f9dc84ba948f8a29 [FIX] missing vars
File Changes
(1 file https://github.com/edickie/ciftify/pull/177/files)
- M ciftify/bin/ciftify_falff.py https://github.com/edickie/ciftify/pull/177/files#diff-6c2042ac314c3ca572599e752206464e5eeeb53cb6fc7cd147543ce9fa2b6554 (116)
Patch Links:
— Reply to this email directly, view it on GitHub https://github.com/edickie/ciftify/pull/177, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADEXT5VP473TVFNZ6AAGLWDWHLNSRANCNFSM6AAAAAAR23BB24 . You are receiving this because your review was requested.Message ID: @.***>
It seems like the main fix happening is the use of repetition time as the second argument to
np.fft.fftfreqon line 178, instead of 1. The two tests intests/functional/test_ciftify_falff.pypass both before and after the changes. Unfortunately, I'm not sure precisely how to update them to capture what these changes are fixing.