-
Notifications
You must be signed in to change notification settings - Fork 14
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
Panel bychr #18
Panel bychr #18
Conversation
Fix #11 |
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.
Good job 🎉 I left some suggestions for further improvement :)
In terms of PRs, I recommend considering the size and focus of future contributions. An example would be separating chromosome handling and the full size dataset changes into distinct PRs for more comprehensive testing and review.
Lastly, it would be useful if these changes were accompanied by updates to the documentation so that it helps with reading and understanding of the changes implemented :)
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 only fix I see is to remove the anyOf
from schema_input_region.json
. Other than that, in principle, LGTM.
Separate panel by chr do the same for the map file.
Update all modules
PR checklist
nf-core lint
).nf-test test main.nf.test -profile test,docker
).nextflow run . -profile debug,test,docker --outdir <OUTDIR>
).docs/usage.md
is updated.docs/output.md
is updated.CHANGELOG.md
is updated.README.md
is updated (including new tool citations and authors/contributors).