Skip to content
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

Moving the RobustICA defaults to a different file #1132

Closed
eurunuela opened this issue Sep 20, 2024 · 4 comments
Closed

Moving the RobustICA defaults to a different file #1132

eurunuela opened this issue Sep 20, 2024 · 4 comments
Labels
discussion issues that still need to be discussed effort: low Theoretically less than a day's work impact: medium Improves code/documentation functionality for some users priority: medium Should get addressed soon

Comments

@eurunuela
Copy link
Collaborator

Summary

The default values of variables needed by RobustICA are currently in tedana/config.py. These are not easily editable by users. We should think of a better approach that would allow users to edit these values when needed.

@eurunuela eurunuela added discussion issues that still need to be discussed priority: medium Should get addressed soon effort: low Theoretically less than a day's work impact: medium Improves code/documentation functionality for some users labels Sep 20, 2024
@BahmanTahayori
Copy link
Contributor

Thanks @eurunuela. I am not sure if I understand this issue. In its current form, the user can change most of the default values from the command line using the available options. The ones that cannot be changed are the min, max and the warning threshold for the number of robust runs and the warning value for the Index Quality. These values are setting the boundaries and is not critical for users to be able to change them.

@handwerkerd
Copy link
Member

I mostly agree with @BahmanTahayori . If you see something a user might want to edit, we can make it a command line option. As an intermediate step, if you think a power user might want to alter any of those lower-level options, we can add them as function parameters and to the API, but not the command line interface. Thoughts?

@eurunuela
Copy link
Collaborator Author

I opened the issue after the discussion we had at the meeting. I understood we wanted to move them elsewhere. Feel free to close this if it's not the case.

@handwerkerd
Copy link
Member

I don't remember or see this in our notes, but that doesn't mean we didn't discuss it. I'm going to close this for now and we can revisit later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion issues that still need to be discussed effort: low Theoretically less than a day's work impact: medium Improves code/documentation functionality for some users priority: medium Should get addressed soon
Projects
None yet
Development

No branches or pull requests

3 participants