Skip to content

Conversation

@alex-sandercock
Copy link
Collaborator

Added a thinSNP() to be able to thin a data.frame of genomic positions using a specified distance

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Jun 4, 2025

Codecov Report

❌ Patch coverage is 72.66187% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.35%. Comparing base (1998376) to head (b98602e).
⚠️ Report is 18 commits behind head on development.

Files with missing lines Patch % Lines
R/madc2gmat.R 72.97% 20 Missing ⚠️
R/updog2vcf.R 16.66% 15 Missing ⚠️
R/madc2vcf_targets.R 90.00% 2 Missing ⚠️
R/thinSNP.R 96.29% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development      #40      +/-   ##
===============================================
- Coverage        84.87%   83.35%   -1.52%     
===============================================
  Files               18       19       +1     
  Lines             1243     1358     +115     
===============================================
+ Hits              1055     1132      +77     
- Misses             188      226      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This comment was marked as outdated.

@alex-sandercock alex-sandercock requested review from Copilot and removed request for josuechinchilla September 9, 2025 18:01
@alex-sandercock alex-sandercock self-assigned this Sep 9, 2025

This comment was marked as outdated.

Copy link
Collaborator

@Cristianetaniguti Cristianetaniguti left a comment

Choose a reason for hiding this comment

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

@alex-sandercock I think you just need to roxygenise to fix the actions error

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new thinSNP function for genomic data processing and makes several improvements to existing functions. The thinSNP function enables filtering SNPs based on minimum genomic distance between markers within chromosomes, which is useful for reducing linkage disequilibrium in genomic analyses.

  • Added thinSNP() function to thin SNP datasets by genomic position and minimum distance
  • Enhanced updog2vcf() function to accept optional reference/alternate allele information
  • Updated madc2gmat() function to support polyploid genomics with VanRaden method implementation

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
R/thinSNP.R New function implementation for SNP thinning based on genomic coordinates
tests/testthat/test-thinSNP.R Test suite for the new thinSNP function
man/thinSNP.Rd Documentation for the thinSNP function
R/updog2vcf.R Enhanced to support optional RefAlt parameter for actual allele information
R/madc2gmat.R Major refactor to support polyploid analysis and VanRaden method
R/breedtools_functions.R Exported previously internal functions for breed composition analysis
NAMESPACE Updated exports and imports for new functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

man/QPsolve.Rd Outdated
Comment on lines 15 to 17
\item{p}{numeric indicating number of breeds represented in X}

\item{names}{character names of breeds}
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

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

The documentation includes parameters p and names that are not present in the actual function signature. The QPsolve function only accepts Y and X parameters.

Suggested change
\item{p}{numeric indicating number of breeds represented in X}
\item{names}{character names of breeds}

Copilot uses AI. Check for mistakes.

test_that("Thinning SNPs",{

##' # Create sample SNP data
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

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

The comment uses Roxygen2 syntax (##') instead of regular R comment syntax (#) in a test file. Test files should use standard R comments.

Suggested change
##' # Create sample SNP data
# Create sample SNP data

Copilot uses AI. Check for mistakes.
@alex-sandercock alex-sandercock merged commit d613ef2 into development Sep 12, 2025
4 of 6 checks passed
@alex-sandercock alex-sandercock deleted the thinning branch September 12, 2025 12:49
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