Skip to content

Conversation

@LiNk-NY
Copy link
Collaborator

@LiNk-NY LiNk-NY commented Nov 6, 2025

Hi Levi, @lwaldron
Could you take a look and provide feedback, if possible?
Thank you!

@LiNk-NY LiNk-NY requested a review from lwaldron November 6, 2025 22:12
@lwaldron
Copy link
Member

The simplifyTCGA(accmae) command in the ?simplifyTCGA examples errors now; trying to understand what's going on...

@LiNk-NY
Copy link
Collaborator Author

LiNk-NY commented Nov 10, 2025

@lwaldron
Sorry, I was not using the default value (hg19) for .get_hsa_gff3. It should be okay now.

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 pull request restores the mirToRanges functionality that was previously marked as defunct. The implementation introduces miRNA name version conversion using the miRNAmeConverter package to handle different miRNA naming versions before mapping them to genomic ranges.

Key changes:

  • Restored mirToRanges function from defunct status with new implementation
  • Added miRNA name conversion logic using miRNAmeConverter package to handle version translations
  • Modified row indexing mechanism to support name remapping when miRNA identifiers are converted between versions

Reviewed Changes

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

File Description
R/simplifyTCGA.R Restored mirToRanges function implementation; added miRNA name conversion logic to .makeListRanges; updated .getRangesOfMir to accept character vector instead of scalar; added conditional rowIdx-based subsetting in .convertTo
DESCRIPTION Added miRNAmeConverter to Suggests dependencies
NAMESPACE Added isCharacter import from BiocBaseUtils
man/simplifyTCGA.Rd Updated author attribution to include M. Ramos

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

R/simplifyTCGA.R Outdated
)
new_x <- trout[[paste0("v", mirna_version, ".0")]]
res <- list(unmapped = setdiff(x, trout[["input"]]))
rowIdx <- match(tolower(trout[["input"]]), x)
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

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

Using tolower(trout[["input"]]) for matching assumes that the original input x contains lowercase miRNA identifiers. However, miRNA identifiers are typically mixed-case (e.g., "hsa-miR-21"). This mismatch will likely cause incorrect row index mapping.

Consider using case-insensitive matching or ensuring consistent case handling: rowIdx <- match(trout[["input"]], x) or rowIdx <- match(tolower(trout[["input"]]), tolower(x)).

Suggested change
rowIdx <- match(tolower(trout[["input"]]), x)
rowIdx <- match(tolower(trout[["input"]]), tolower(x))

Copilot uses AI. Check for mistakes.
@LiNk-NY LiNk-NY merged commit 140490c into devel Nov 11, 2025
3 of 4 checks passed
@LiNk-NY LiNk-NY deleted the gff3 branch December 11, 2025 21:57
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