Add isolated_as_missing to ts.alignements #3294
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3294 +/- ##
==========================================
+ Coverage 89.65% 89.78% +0.13%
==========================================
Files 29 29
Lines 30607 30964 +357
Branches 5614 5665 +51
==========================================
+ Hits 27441 27802 +361
+ Misses 1780 1779 -1
+ Partials 1386 1383 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
This is definitely a big help, but we should clarify how this relates to #1896 and what's going on more generally.
| (default) alignments will continue until the end of the tree sequence. | ||
| :param bool isolated_as_missing: If True (default), isolated samples without | ||
| mutations are treated as missing data and this method raises an error | ||
| when any are detected. If False, missing data are imputed with the |
There was a problem hiding this comment.
Suggested change
| when any are detected. If False, missing data are imputed with the | |
| when any are detected. If False, missing data is imputed with the |
python/CHANGELOG.rst
Outdated
|
|
||
| - ``TreeSequence.alignments`` now accepts ``isolated_as_missing=False`` so that | ||
| alignments can be emitted for non-sample nodes (e.g., internal ARG nodes) when | ||
| missing data are imputed to the ancestral state. (:user:`benjeffery`, :issue:`3293`) |
There was a problem hiding this comment.
Suggested change
| missing data are imputed to the ancestral state. (:user:`benjeffery`, :issue:`3293`) | |
| missing data is imputed to the ancestral state. (:user:`benjeffery`, :issue:`3293`) |
| tree._has_isolated_samples() for tree in self.trees() | ||
| ): | ||
| raise ValueError( | ||
| "Missing data not currently supported in alignments; see " |
There was a problem hiding this comment.
Let's update this to suggest using isolated_as_missing, but note some caveats?
benjeffery
force-pushed
the
internal-alignments
branch
from
c343634 to
c3dcdcf
Compare
|
I've updated to add more detail in the docs and error. |
|
Looks good. Can you change the "data are" to "data is" please? (This is a hill I will die on!) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3293
This seemed to be quite straight-forward? Maybe when we added
isolated_as_missingwe just didn't apply it toalignements?