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

tedpca CLI fix #950

Merged
merged 2 commits into from
Jun 7, 2023
Merged

tedpca CLI fix #950

merged 2 commits into from
Jun 7, 2023

Conversation

handwerkerd
Copy link
Member

@handwerkerd handwerkerd commented Jun 6, 2023

Closes #949.

Changes proposed in this pull request:

  • Fixes a bug where tedpca can accept several strings or numbers as inputs, but the argument parser for the command line interface (CLI) rejected any option except aic mdl or kic.
  • Also cleans up some language in docs and comments

@BahmanTahayori Can you check to confirm this resolves your issue?

@handwerkerd handwerkerd changed the title tedpca CLI fix and new option tedpca CLI fix Jun 6, 2023
@handwerkerd handwerkerd mentioned this pull request Jun 6, 2023
Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

Looks good to me. check_tedpca_value should be able to validate the value.

@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (9a3b83f) 88.97% compared to head (f304e20) 88.97%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #950   +/-   ##
=======================================
  Coverage   88.97%   88.97%           
=======================================
  Files          27       27           
  Lines        3373     3373           
  Branches      617      617           
=======================================
  Hits         3001     3001           
  Misses        226      226           
  Partials      146      146           
Impacted Files Coverage Δ
tedana/decomposition/pca.py 76.61% <ø> (ø)
tedana/workflows/tedana.py 80.95% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

LGTM!

@tsalo tsalo added the bug issues describing a bug or error found in the project label Jun 6, 2023
@BahmanTahayori
Copy link
Contributor

Thanks, Dan and the team,

Yes, that is what I did on my venv as well. Thanks for fixing this issue quickly.

@handwerkerd handwerkerd merged commit 18e950d into ME-ICA:main Jun 7, 2023
@handwerkerd handwerkerd deleted the tedpcaCLIfix branch June 7, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues describing a bug or error found in the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with tedpca
4 participants