-
Notifications
You must be signed in to change notification settings - Fork 95
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
[ENH] Allow tedpca argument to be a float inside a string #665
Conversation
This is definitely a much cleaner approach. It looks like there are style issues that need to be addressed and I might have some changes to the whitespace to request, but overall this looks great! |
Codecov Report
@@ Coverage Diff @@
## master #665 +/- ##
==========================================
- Coverage 93.75% 93.75% -0.01%
==========================================
Files 26 26
Lines 2019 2017 -2
==========================================
- Hits 1893 1891 -2
Misses 126 126
Continue to review full report at Codecov.
|
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.
LGTM!
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.
One minor change for readability.
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.
The changes look great!
Closes none.
Got a parser error while trying the variance-based PCA introduced in #658.
I think it's because the inputs to
workflows.tedana.main_
are strings but the parser treats"0.95"
as an invalid input.This PR modifies
workflows.parser_utils.check_tedpca_value
to accept floats-inside-strings. The function got refactored a bit while I was debugging.