Skip to content

Conversation

@LouisLeNezet
Copy link

This PR add bam and vcf file for two individuals on two chromosomes to simulate low-pass data.
Genetic map in various format for the different imputation tools are also added as well as the reference panel for the two chromosomes.

Copy link
Member

@JoseEspinosa JoseEspinosa left a comment

Choose a reason for hiding this comment

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

LGTM!

@LouisLeNezet LouisLeNezet merged commit 1d55643 into nf-core:modules Nov 26, 2025
@mashehu
Copy link
Contributor

mashehu commented Nov 26, 2025

hmm, this broke some tests that relied on the old file names, e.g. https://github.com/nf-core/modules/actions/runs/19709033762/job/56464494808?pr=9333#step:6:848

@LouisLeNezet
Copy link
Author

I'm currently updating them

@mashehu
Copy link
Contributor

mashehu commented Nov 27, 2025

I think the easier move would be to add the the files without removing old files. Or was there something wrong with them?

@LouisLeNezet
Copy link
Author

The aim was to avoid to have the same file with two or only one chromosome.
The genetic map was also not properly formatted.

Choose a reason for hiding this comment

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

@LouisLeNezet , why was this deleted?
This has broken sarek tests:
https://github.com/nf-core/sarek/actions/runs/19706542568/job/56455675487?pr=2060#step:4:1381
as well as presumably two modules (cnvnator/cnvnator and cnvnator/convert2vcf)

Copy link
Contributor

Choose a reason for hiding this comment

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

should we just add the old files back (with a fixed map)?

Copy link
Author

Choose a reason for hiding this comment

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

I'm really sorry. That it broke sarek pipeline.
I will add back the chr22 only files.

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the two other modules.

Choose a reason for hiding this comment

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

thank you! don't want to rewrite all tests for the upcoming release

Copy link
Author

Choose a reason for hiding this comment

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

Sorry again 😞

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Done in #1803

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

5 participants