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

added default -doMajorMinor 1 to output angsd beagle format #690

Merged
merged 5 commits into from
Mar 10, 2021

Conversation

alexandregilardet
Copy link
Contributor

  • corrected typo in error message about angsd output format

To output ANGSD beagle format, the argument -doMajorMinor to infer major and minor alleles is required. For now, eager would output a corrupted .glf.gz file if 'beagle' is selected as output with the following .command.err:
-> For dumping beaglestyle output you need to estimate major/minor: -doMajorMinor
I added as default the -doMajorMinor 1 argument which I'm more familiar with (http://www.popgen.dk/angsd/index.php/Major_Minor).

PR checklist

  • This comment contains a description of changes (with reason).
  • Make sure your code lints (nf-core lint .).
  • Ensure the test suite passes (nextflow run . -profile test_tsv,crick).

…ed typo in error message about angsd output format
@apeltzer
Copy link
Member

apeltzer commented Mar 8, 2021

You will need to add this change to the CHANGELOG.md too to make the tests pass 👍🏻 Otherwise makes sense to add this to make it more robust against failure and corrupted output is a clear no-no - thanks for catching this!

@alexandregilardet
Copy link
Contributor Author

Ok I THINK I got it right now...

Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Good catch on the typos in the validation section.

There was a mistake in the changelog I think, but otherwise I think we are good to go.

CHANGELOG.md Outdated
Comment on lines 12 to 23
- [#666](https://github.com/nf-core/eager/issues/666) - Fixed input file staging for `print_nuclear_contamination`
- [#631](https://github.com/nf-core/eager/issues/631) - Update minimum Nextflow version to 20.07.1, due to unfortunate bug in Nextflow 20.04.1 causing eager to crash if patch pulled
- Made MultiQC crash behaviour stricter when dealing with large datasets, as reported by @ashildv
- [#652](https://github.com/nf-core/eager/issues/652) Added note to documentation that when using `--skip_collapse` this will use _paired-end_ alignment mode with mappers when using PE data
- [#626](https://github.com/nf-core/eager/issues/626) - Add additional checks to ensure pipeline will give useful error if cells of a TSV column are empty
- Added note to documentation that when using `--skip_collapse` this will use _paired-end_ alignment mode with mappers when using PE data. `
- [#673](https://github.com/nf-core/eager/pull/673) Fix Kraken database loading when loading from directory instead of compressed file.
- [#688](https://github.com/nf-core/eager/issues/668) - Allow pipeline to complete, even if Qualimap crashes due to an empty or corrupt BAM file for one sample/library
- [#683](https://github.com/nf-core/eager/pull/683) - Sets `--igenomes_ignore` to true by default, as rarely used by users currently and makes resolving configs less complex.
- Added exit code `140` to re-tryable exit code list to account for certain scheduler wall-time limit fails.
- [672](https://github.com/nf-core/eager/issues/672) - Removed java parameter from picard tools which could cause memory issues
- [679](https://github.com/nf-core/eager/issues/679) - Refactor within-process bash conditions to groovy/nextflow, due to incompatibility with some servers environments
Copy link
Member

Choose a reason for hiding this comment

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

Was this all meant to be deleted? Your change should just be appened to the endof the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I got a bit confused because I was on my master branch so I didn't have the dev section in the CHANGELOG.md

@jfy133
Copy link
Member

jfy133 commented Mar 10, 2021

Looks good to me! Merging!

@jfy133
Copy link
Member

jfy133 commented Mar 10, 2021

Thanks very much @alexandregilardet ! Feel free to make an extra PR adding yourself to the contributors list on the main README if you're happy to do so :)

@jfy133 jfy133 merged commit 446fd49 into nf-core:dev Mar 10, 2021
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

Successfully merging this pull request may close these issues.

3 participants