Skip to content

Add variation of Jukes-Cantor distance - #61

Merged
charles-plessy merged 3 commits into
mainfrom
jc69_fullseq
Aug 27, 2026
Merged

Add variation of Jukes-Cantor distance#61
charles-plessy merged 3 commits into
mainfrom
jc69_fullseq

Conversation

@pribiller

Copy link
Copy Markdown
Contributor

This pull request adds a variation of the Jukes-Cantor distance that takes into account unaligned base pairs in the computation.
It also updates the documentation of existing functions from the family "Alignment statistics" and "Similarity indexes".

Add variation of Jukes-Cantor distance. The Jukes-Cantor equation remains unchanged,
but the definition of which base pairs are considered different is modified.
In this function, the fraction of nucleotides that are different incorporates not
only the mismatches from the alignment matrix but also the base pairs that are left unaligned.
Add documentation for new function and update documentation of existing functions
from the family "Alignment statistics" and "Similarity indexes".

@charles-plessy charles-plessy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can I ask you two minor changes?

Comment thread R/JC69_distance_allseq.R
#' d <- JC69_distance_allseq(gb, exampleSubstitutionMatrix)
#'
#' @export
JC69_distance_allseq <- function(gb, m, adjust_p=FALSE) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you add the usual guard for object length 0 ?

  if(length(gb) == 0) return(numeric(0))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment thread R/JC69_distance_allseq.R Outdated
# Gets the smallest sequence length.
totBps <- min(sum(seqlengths(gb)),sum(seqlengths(gb$query)))
if(is.na(totBps)){
totBps <- min(sum(guessSeqLengths(gb)),sum(guessSeqLengths(gb$query)))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

guessSeqLengths always returns seqlengths when available, so you can run totBps <- min(sum(guessSeqLengths(gb)),sum(guessSeqLengths(gb$query))) straight. Maybe I could have called the function safeSeqLengths but it is probably too late to rename it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Add safeguard for gb object
Remove redundant code
@pribiller

Copy link
Copy Markdown
Contributor Author

Thanks Charles for your comments! I have updated the code with the proposed changes.

@charles-plessy
charles-plessy merged commit cbe0777 into main Aug 27, 2026
1 check passed
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.

2 participants