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

First speed pass #56

Closed
6 tasks done
ewels opened this issue Jun 30, 2023 · 10 comments
Closed
6 tasks done

First speed pass #56

ewels opened this issue Jun 30, 2023 · 10 comments

Comments

@ewels
Copy link
Member

ewels commented Jun 30, 2023

First couple of quick things from looking over the files, without actually reading the real code...

Please delete these files:

  • CODEOWNERS
  • samplesheet.transformed.csv
    • Is this some kind of testing data? Needs to go into nf-core/test-datasets if so

Minor changes:

Questions

  • What is this file for? Apologies if I missed some docs etc, just glancing at the files right now.
@fbdtemme
Copy link
Collaborator

samplesheet.transformed.csv was a temp file that sneaked into git.
I have removed the files and fixed the markdown rendering error.

For the homepage you mean: https://github.com/nf-core/pixelator right?
nf-core lint complains otherwise.

The create-params-template.py file is used to create this file in ./assets/. It is a template to pass to -params-file but with everthing commented out and descriptions and defaults taken from the pipeline schema. It only includes the pixelator specific options.

It can be handy to copy to your job working dir and start editing when you are in a remote machine and do not want to use the nf-core launch terminal interface.

@ewels
Copy link
Member Author

ewels commented Jun 30, 2023

I have removed the files and fixed the markdown rendering error.

Brilliant, thanks!

For the homepage you mean: https://github.com/nf-core/pixelator right?

Right, sorry. Yes - whatever gets linting to pass without ignoring.

and do not want to use the nf-core launch terminal interface.

Who would not want to use such a wonderful thing?! 😭 ok, fair enough, I see how the YAML file is nice to work with 😅

This looks like it should be fairly generic, it's just rendering from the Nextflow schema file, right? What do you think about breaking it out into a subcommand of nf-core/tools? It could be useful for most Nextflow pipelines I think. That would help to keep the pipeline repo clean and also make the tool available to everyone..

The groups seem to be hardcoded, any reason for that?

@fbdtemme
Copy link
Collaborator

fbdtemme commented Jul 3, 2023

A subcommand for nf-core tools would be cool. The groups were hard coded to keep the options limited to the actual pixelator specific flags and thus make the file shorter by not including descriptions and default for all the boilerplate parameters.
To make it generic for nf-core/tools we can definitely drop the hard-coded parameter groups without issues.

@ewels
Copy link
Member Author

ewels commented Jul 3, 2023

Great! Do you fancy either making a PR to nf-core/tools, or putting the script into a GitHub gist etc. and opening an issue? Ideally we can get this migrated soon and remove it from the pipeline before release..

@ewels
Copy link
Member Author

ewels commented Jul 3, 2023

make the file shorter by not including descriptions and default for all the boilerplate parameters.

Can maybe do the same thing by ignoring everything that has hidden: true in the schema? 🤔 Not sure if there are any that need to stay unhidden but should not be in the params.yml file though..

@fbdtemme
Copy link
Collaborator

fbdtemme commented Jul 3, 2023

Yes, that makes sense. I'll make a PR.

@fbdtemme
Copy link
Collaborator

fbdtemme commented Jul 4, 2023

You can find the PR here: nf-core/tools#2362

@ewels
Copy link
Member Author

ewels commented Aug 9, 2023

Just waiting for utils/create-params-template.py to be deleted, then this issue can be closed 👍🏻

@fbdtemme
Copy link
Collaborator

fbdtemme commented Aug 9, 2023

Right, forgot about that. It is gone now: PixelgenTechnologies@e9cbb60

@fbdtemme fbdtemme closed this as completed Aug 9, 2023
@ewels
Copy link
Member Author

ewels commented Aug 9, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants