-
Notifications
You must be signed in to change notification settings - Fork 201
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
Feature/test datasets #3494
base: dev
Are you sure you want to change the base?
Feature/test datasets #3494
Conversation
…tch RequestExceptions
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.
I made some comments especially about the structure of the files, but it is looking good! 😄
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.
To follow the same structure as in other commands, the main functions of each sub-command should be moved to its own script. utils
should only contain the functions that are common to all these scripts.
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.
I restructured all logic into search.py
and list.py
👍 But I would suggest keeping all functions in utils.py
in the same plce, as they are either exported by more than one other file, or depend on each other.
And one last comment. When we print the list of files or branches, we could use a rich table like we do here |
Noooo no rich stuff!!!!! Makes it much harder to copy and paste 😭🫠🫠😭🫠😭🫠😭 |
I'm on the fence on this one. Usually I also prefer plain text output because it is just easier to pipe, but rich is used a lot in nf-core, right? |
Latter is probably best I think, as makes me and you happy but default sticks with the typical nf-core convention |
Oh and one more general question: Whats our stance on type hints @mirpedrol ? Should I add them? |
We are slowly adopting them when we add new code or modify old code, so yes, you should add them 😄 |
…for easier testing
…re-tools into feature/test-datasets
This is a DRAFT.This PR adds functionality to list and search the nf-core/test-datasets github repository via a new subcommand
test-datasets
. Closes #3487 where this feature was requested.Changes:
test-datasets search
,test-datasets list
,test-datasets list-branches
test-datasets
folder with command and utility functionsPR checklist
CHANGELOG.md
is updateddocs
is updated