-
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
Move WorkflowRepo class into seperate file, since it will be shared b… #3506
base: dev
Are you sure you want to change the base?
Move WorkflowRepo class into seperate file, since it will be shared b… #3506
Conversation
…y container handlers as well.
Codecov ReportAttention: Patch coverage is
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 name of the directory should be in singular as we do with the other commands nf_core/pipelines/download
And since we are creating this new folder, the script nf_core/pipelines/download.py
should also be moved here
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 reason for it to be plural was that the main file is called download.py
. This ambiguity resulted in errors like
ModuleNotFoundError: No module named 'nf_core.pipelines.download.singularity'; 'nf_core.pipelines.download' is not a package
ImportError: cannot import name 'DownloadWorkflow' from 'nf_core.pipelines.download'
Calling the directory downloads
therefore was a straightforward way to avoid ambiguity and avoid name collisions. By moving the main file inside the directory, of course, it would be possible to resolve that as well.
Regarding the plural, I rather think we should reconsider moving it under pipelines in the first place. After all, we will eventually have separate commands for nf-core download(s) container
(#2408) and nf-core download(s) pipelines
and nf-core download(s)
plugins (#3344) commands.
The name for tools 2.x was obviously nf-core download
, but if you prefer to have it consistent with modules
, subworkflows
or pipelines
turning it into the downloads
plural would work as well, I guess.
stderr = rich.console.Console( | ||
stderr=True, | ||
style="dim", | ||
highlight=False, | ||
force_terminal=nf_core.utils.rich_force_colors(), | ||
) |
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.
This is not used here
stderr = rich.console.Console( | |
stderr=True, | |
style="dim", | |
highlight=False, | |
force_terminal=nf_core.utils.rich_force_colors(), | |
) |
Move WorkflowRepo class into separate file, since it will be shared with container handlers as well.
PR checklist
CHANGELOG.md
is updateddocs
is updated