Pedigree phasing with whatshap (issue 387) by tetedange13 · Pull Request #968 · genomic-medicine-sweden/nallo

tetedange13 · 2026-03-27T01:27:08Z

Description

Should adress issue #387

Changed

When phasing with whatshap + full family (proband + both parents), now use pedigree information to phase

Main steps

In nallo.nf, apply addChildWithTwoParentsToMeta function on "by family pedigree" channel and set pedigree_file to "empty" if family not complete
Pass this new pedigree tuple through phasing then whatshap subworkflows
In subworkflows/whatshap, use same approach to sync PED with BAM+VCF (otherwise PED files could be swapped between 2 different families)
PED goes up to nf-core/whatshap/phase module, that I patched to add a new ch_pedigree input

Warnings / limitations

2 spurious commits = snapshot changes that seem due to other unrelated changes of code : 853517b and a9e1a98 (parts are related to my changes and others not)
No test in subworkflows/whatshap to capture this new behaviour ? Would require 2 trios, or best 2 trios + 1 solo ? Is there enough test data on genomic-medicine-sweden/test-datasets for that ?
A new test sampleSheet with 2 trios + 1 solo could be a good add too ?
nf-core modules lint whatshap/phase not passing anymore, even if I used nf-core modules patch. Complains about meta.yml missing new pedigree input, even if it is well present...

Thanks for taking a look !
Best,
Felix.

Default to 'none' for now

Otherwise wrong PED could be used when multiple families

Simplify tests update

Should not be impacted by change in whatshap

Separate commit because parts of snapshot changes seem to come from other unrelated commits (eg: PR 948 ?)

github-actions · 2026-03-27T01:27:27Z

PR checklist

Please describe the purpose of this change, the problem it solves, why this approach was chosen, its impact, and link any relevant issue if it already contains this information.
If you've fixed a bug or added code that should be tested, add tests!
If you've added a new tool, follow the pipeline conventions in the contribution docs.
Usage Documentation in docs/usage.md is updated.
Output Documentation in docs/output.md is updated.
README.md is updated (including new tools and authors/contributors).

fellen31

Thanks a lot for this contribution!

No test in subworkflows/whatshap to capture this new behaviour ? Would require 2 trios, or best 2 trios + 1 solo ? Is there enough test data on genomic-medicine-sweden/test-datasets for that ?

New tests would be great! And I think so! Since whatshap phases one family at a time, we should be able to reuse the input files but with different family/sample IDs. So you could probably use the PacBio HG002, HG003 and HG004 files twice for trios, and then pick one of the files for an extra solo.

A new test sampleSheet with 2 trios + 1 solo could be a good add too ?

I think if you can add 2 trios + 1 solo in the subworkflow test, we should be fine with the current tests. The only logic changed in nallo.nf is whether or not we pass the PED-file right?

I was also wondering if we should have a parameter controlling whether or not PED phasing is applied for trios...but I guess there's no downside to it, so maybe that's just unnecessary?

Would be interesting to hear if phasing a trio with whatshap ped is better than e.g. longphase. Have you tried?

fellen31 · 2026-03-27T08:44:03Z

modules/nf-core/whatshap/phase/main.nf

This update would be great otherwise have in the nf-core module. If you make a PR to nf-core/modules with these updates you can ping me for a review in the nf-core Slack. Should also take care of issues with the linting.

Still applies.

Latest version of whatshap/phase module changed input structure : https://github.com/nf-core/modules/pull/11041/changes#diff-04d6148de5293b82d05a67c61d9995e47c020e76e0f16f950dac131382e62e62

But I am on it !

PR to add pedigree input to nf-core/whatshap/phase : nf-core/modules#11128

EDIT : Will probably take me longer than expected to adress this PR, so in the meantime I updated whatshap/phase (with new inputs structure) and re-patched it with ped input

subworkflows/local/call_methylation_methbat/tests/main.nf.test

subworkflows/local/call_methylation_modkit/tests/main.nf.test

subworkflows/local/whatshap/tests/main.nf.test

subworkflows/local/whatshap/main.nf

workflows/nallo.nf

fellen31 · 2026-03-27T08:58:58Z

workflows/nallo.nf

+                .map { meta, _files -> [ [ id: meta.family_id ], meta ] }
+                .groupTuple()
+        )
+        // If 'childWithTwoParents==false', set family_ped=empty


Nice, but could you explain why we need to do this? Will whatshap fail if you give it a PED with only two individuals in a family?

Suggested change

// If 'childWithTwoParents==false', set family_ped=empty

// If childWithTwoParents is false we don't provide the ped file, because ...

Based on Whatshap documentation, pedigree phasing activates a distinct algorithm

So I enforced it to "full trio", in one hand to not risk worse results on cases like "duos" or even "solos". And in the other hand to reserve pedigree phasing for when it is most relevant (= when we have both parents I guess)

But I did actually made a benchmark of all that, to be honest

Then I would write something like "If we provide a PED file, whatshap phase will automatically activate the pedigree phasing algorithm which is only available for trios..", to make it more clear.

However, now that you link the documentation it seems like it also supports quartets (which is fine - childWithTwoParents will be true for both trios and quartets), but also duos (parent/childs):

A quartet (note how multiple consecutive spaces are fine):

When phasing multiple samples from individuals that are related (such as parent/child or a trio), then it is possible to provide WhatsHap with a .ped file that describes the pedigree. WhatsHap will use the pedigree and the reads to infer a combined, much better phasing.

Based on this line in the WhatsHap code: https://github.com/whatshap/whatshap/blob/f07423a7ba0c7609e7d4d5c73fb5e4e240057830/whatshap/cli/phase.py#L580, it seems to me like it would just ignore the PED file for singletons. So maybe we could always provide the PED file?

If we are want to be able to control the algorithm when we have parent/child, perhaps we could have a pipeline parameter, e.g. whatshap_pedigree_phasing where the user can determine if we provide the PED file or not? (edit: That would default to true)

Adressed by commit abc28d0 (and 106a2ca)

Commit 7930be2 fix an important typo between param_name and val_param_name...

workflows/nallo.nf

modules/local/sentieon/dnascope_longread/tests/main.nf.test.snap

Pass PED file in one of them, to test new 'whatshap pedigree phasing modifié : subworkflows/local/whatshap/tests/main.nf.test modifié : subworkflows/local/whatshap/tests/main.nf.test.snap

This reverts commit 6512c02.

Pass PED file in one of them, to test new 'whatshap pedigree phasing

…ng false'

+ Forgotten debug 'view()'

fellen31

Nice!

fellen31 · 2026-04-01T15:53:54Z

subworkflows/local/whatshap/tests/main.nf.test

+                input[5] = channel.of([
+                    [ id:'FAM' ], // Empty ch_pedigree
+                    []
+                ])


Could we try inputting the PED-file here as well?

Smart idea testing that !

It works, but snapshots do not match anymore

However I think this a false positive, simply due to use of simple snapshot() instead of dedicated nft-vcf plugin's variantsMD5() (by manual check VCFs seem identical, except for add of --ped FAM.ped in header)

Plot twist is that looking at logs when inputting PED :

I see Using uniform recombination rate of 1.26 cM/Mb. (absent when no PED), probably due to https://github.com/whatshap/whatshap/blob/f07423a7ba0c7609e7d4d5c73fb5e4e240057830/whatshap/cli/phase.py#L440, where len(family) is not checked

But at the same time solving MEC problem (contrary to PedMEC problem when inputting PED with trio)

So I guess that PED is well ignored when given on a solo

I also tested on a bigger dataset (100 K variants) and whatshap phase still produces identical results on a solo with or without --ped option --> Confirm we should be fine, PED is well ignored with solo
(even I cannot exactly pinpoint this behaviour in whatshap phase's code)

Do we keep it like that or do you prefer to play safe and get back to initial "phasing for complete trios ONLY" option ?

subworkflows/local/whatshap/tests/main.nf.test

tests/samplesheet_stub.nf.test.snap

workflows/nallo.nf

fellen31 · 2026-04-01T16:14:35Z

workflows/nallo.nf

            val_phaser,
            !val_skip_sv_calling,
            cram_output,
+            ch_ped_family


I think we could always pass the ped file here (unless we do not want pedigree phasing), even though it will only be used by whatshap.

I prefer always passing the meta along with the file, the empty equivalent would be [[],[]]

We recently moved all params from this workflow to main.nf, since this will be required in future versions of Nextflow. Could you make a val_whatshap_pedigree_phasing input to nallo.nf?

Suggested change

ch_ped_family

val_whatshap_pedigree_phasing ? SOMALIER_PED_FAMILY.out.ped : [[],[]]

Adressed by commits 8bb9d80 and 8a7b339

Commit 8a7b339 was abusive : you have to use an intermediate variable with a correct meta, otherwise --whatshap_pedigree_phasing false raises a join mismatch error there : https://github.com/tetedange13/nallo/blob/7930be21c4c7a13a61a5232f1586eae4c7153307/subworkflows/local/whatshap/main.nf#L27

Fixed by commit 0a5a6d2

subworkflows/local/phasing/main.nf

fellen31 · 2026-04-01T16:15:51Z

modules/nf-core/whatshap/phase/main.nf

Still applies.

docs/parameters.md

Merge conflicts each time..

Even if only applied when '--phaser whatshap'

+ put it inside 'if' block

fellen31 · 2026-04-02T13:28:02Z

I'll be away for a few weeks but I'll check in again when I get back, unless someone else can continue the review @tetedange13!

To start working with new input structure

felix.vandermeeren@chu-montpellier.fr added 13 commits March 17, 2026 17:10

[MODULES] Patch whashap_phase to add PED input

b97fb72

Pass new PED input to WhatsHap

f9028c4

Default to 'none' for now

Forgot to remove debug '.view('

34cd5d5

Restrict PED-phasing to 'complete' trios

5bf3332

FIX: Sync PED with BAM and VCF

c1133b3

Otherwise wrong PED could be used when multiple families

Put pedigree as last input

9608bd7

Simplify tests update

Adapt 'nf.test' with new input 'ch_pedigree'

10dfbf4

Update snapshots after adding new input 'ch_pedigree'

2a154b7

nf-test update forgotten by commit 5e258f8 ?

853517b

Should not be impacted by change in whatshap

Merge dev into 387-ped-phasing

984a3d8

Forgot to resolve some conflicts

6193c52

Update snapshots after whatshap trio phasing

bc649a5

Update snapshots after whatshap trio phasing (2)

a9e1a98

Separate commit because parts of snapshot changes seem to come from other unrelated commits (eg: PR 948 ?)

tetedange13 requested a review from a team as a code owner March 27, 2026 01:27

github-project-automation bot added this to Nallo Mar 27, 2026

fellen31 reviewed Mar 27, 2026

View reviewed changes

felix.vandermeeren@chu-montpellier.fr added 2 commits March 27, 2026 10:31

Update CHANGELOG

3f35d8a

Turn '2 samples' into '3'

6512c02

Pass PED file in one of them, to test new 'whatshap pedigree phasing modifié : subworkflows/local/whatshap/tests/main.nf.test modifié : subworkflows/local/whatshap/tests/main.nf.test.snap

tetedange13 force-pushed the 387-ped-phasing2 branch from 569b2a8 to 6512c02 Compare March 27, 2026 15:19

felix.vandermeeren@chu-montpellier.fr added 2 commits March 27, 2026 16:21

Revert "Turn '2 samples' into '3'"

a3450f4

This reverts commit 6512c02.

Turn '2 samples' into '3'

dd00f35

Pass PED file in one of them, to test new 'whatshap pedigree phasing

tetedange13 changed the title ~~387 ped phasing2~~ Pedigree phasing with whatshap (issue 387) Mar 27, 2026

felix.vandermeeren@chu-montpellier.fr added 7 commits March 30, 2026 14:10

Re-use SOMALIER_PED_FAMILY instead of new one

e4858e7

CHANGE: Always pass PED unless (new param) '--whatshap_pedigree_phasi…

abc28d0

…ng false'

FIX: forgot to include new param at prev commit

106a2ca

FIX: 'addChildWithTwoParentsToMeta()' call not needed

6df963f

+ Forgotten debug 'view()'

Precise default behaviour

b2adf17

Merge dev into branh

3e529ae

Update snapshots

ebf0235

Merge branch 'dev' into 387-ped-phasing2

f9363df

fellen31 reviewed Apr 1, 2026

View reviewed changes

felix.vandermeeren@chu-montpellier.fr added 6 commits April 2, 2026 00:20

Merge dev

17cbc43

Will update at the end

30f3363

Merge conflicts each time..

Move new param to main

8bb9d80

Always pass ch_pedigree

8a7b339

Even if only applied when '--phaser whatshap'

Fix new param description

22f287a

Prev codeblock position did not work when '--skip_align'

91954d1

+ put it inside 'if' block

felix.vandermeeren@chu-montpellier.fr added 10 commits April 7, 2026 10:48

FIX: 'join mismatch' with empty tuple

0a5a6d2

FIX: typo 'param_name' vs 'val_param'

7930be2

Update snapshots

1cb57e4

Merge dev

a403332

FIX: prev merge introduced duplicate code

d2a5351

Forgot to remove commented code

cb97a40

FIX: Forgotten TODO

d110cfc

Update whatshap module

09b753c

To start working with new input structure

Re-patch whatshap/phase with ped input

915f958

Adapt subwf to new whatshap/phase module

9b07125

	// If 'childWithTwoParents==false', set family_ped=empty
	// If childWithTwoParents is false we don't provide the ped file, because ...

	ch_ped_family
	val_whatshap_pedigree_phasing ? SOMALIER_PED_FAMILY.out.ped : [[],[]]

Conversation

tetedange13 commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changed

Main steps

Warnings / limitations

Uh oh!

github-actions bot commented Mar 27, 2026

PR checklist

Uh oh!

fellen31 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetedange13 Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetedange13 Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fellen31 Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetedange13 Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fellen31 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetedange13 Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetedange13 Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetedange13 Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fellen31 commented Apr 2, 2026

Uh oh!

tetedange13 commented Mar 27, 2026 •

edited

Loading

tetedange13 Apr 7, 2026 •

edited

Loading

tetedange13 Mar 27, 2026 •

edited

Loading

fellen31 Mar 27, 2026 •

edited

Loading

tetedange13 Apr 7, 2026 •

edited

Loading

tetedange13 Apr 7, 2026 •

edited

Loading

tetedange13 Apr 8, 2026 •

edited

Loading

tetedange13 Apr 1, 2026 •

edited

Loading