Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Externalize .LevelReduction and .GetKingdom for global use #85

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.Rproj.user
docs
.Rhistory
.DS_Store
81 changes: 41 additions & 40 deletions R/plotting.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,39 @@
# suppressPackageStartupMessages(library(d3r))
# suppressPackageStartupMessages(library(viridis))

########################
## Internal Functions ##
########################
#'
#'
.LevelReduction <- function(lin, level) {
gt_loc <- str_locate_all(lin, ">")[[1]]
available_levels <- length(gt_loc) / 2 # Since `str_locate_all` returns a matrix

# Guard against out-of-bounds level requests
if (level > available_levels || level < 1) {
return(lin)
} else {
gt_loc <- gt_loc[level, ][1] %>% as.numeric()
lin <- substring(lin, first = 0, last = (gt_loc - 1))
return(lin)
}
}



.GetKingdom <- function(lin) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Function Modularization:

Functions .LevelReduction and .GetKingdom, initially defined within plotLineageHeatmap, are moved outside and set as independent internal functions. This modularization improves reusability, readability, and reduces nested function complexity within plotLineageHeatmap.

gt_loc <- str_locate(lin, ">")[, "start"]
if (is.na(gt_loc)) {
# No '>' in lineage
return(lin)
} else {
kingdom <- substring(lin, first = 0, last = (gt_loc - 1))
return(kingdom)
}
}


#' Shorten Lineage
#'
#' @param data
Expand Down Expand Up @@ -665,30 +698,6 @@ plotLineageDomainRepeats <- function(query_data, colname) {
#' }
#'
plotLineageHeatmap <- function(prot, domains_of_interest, level = 3, label.size = 8) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

A fallback is added for color assignment in plotLineageHeatmap, defaulting to black if there are insufficient colors for the unique lineages. This ensures visual consistency without missing colors in the output.

.LevelReduction <- function(lin) {
if (level == 1) {
gt_loc <- str_locate(lin, ">")[[1]]
if (is.na(gt_loc)) {
# No '>' in lineage
return(lin)
} else {
lin <- substring(lin, first = 0, last = (gt_loc - 1))
return(lin)
}
}
#### Add guard here to protect from out of bounds
gt_loc <- str_locate_all(lin, ">")[[1]] # [(level-1),][1]
l <- length(gt_loc) / 2
if (level > l) {
# Not enough '>' in lineage
return(lin)
} else {
gt_loc <- gt_loc[level, ][1] %>% as.numeric()
lin <- substring(lin, first = 0, last = (gt_loc - 1))
return(lin)
}
}

all_grouped <- data.frame("Query" = character(0), "Lineage" = character(0), "count" = integer())
for (dom in domains_of_interest)
{
Expand All @@ -703,19 +712,7 @@ plotLineageHeatmap <- function(prot, domains_of_interest, level = 3, label.size
all_grouped <- dplyr::union(all_grouped, domSub)
}

.GetKingdom <- function(lin) {
gt_loc <- str_locate(lin, ">")[, "start"]

if (is.na(gt_loc)) {
# No '>' in lineage
return(lin)
} else {
kingdom <- substring(lin, first = 0, last = (gt_loc - 1))
return(kingdom)
}
}

all_grouped <- all_grouped %>% mutate(ReducedLin = unlist(purrr::map(Lineage, .LevelReduction)))
all_grouped <- all_grouped %>% mutate(ReducedLin = unlist(purrr::map(Lineage, ~.LevelReduction(.x, level))))

all_grouped_reduced <- all_grouped %>%
group_by(Query, ReducedLin) %>%
Expand All @@ -739,6 +736,10 @@ plotLineageHeatmap <- function(prot, domains_of_interest, level = 3, label.size
append(eukaryota_colors) %>%
append(virus_colors)

if (length(colors) < length(unique(all_grouped_reduced$ReducedLin))) {
colors <- rep("black", length(unique(all_grouped_reduced$ReducedLin))) # Fallback to black
}

all_grouped_reduced$ReducedLin <- map(
all_grouped_reduced$ReducedLin,
function(lin) {
Expand Down Expand Up @@ -766,15 +767,15 @@ plotLineageHeatmap <- function(prot, domains_of_interest, level = 3, label.size
)
ggplot(
data = all_grouped_reduced,
aes_string(x = "ReducedLin", y = "Query")
aes(x = "ReducedLin", y = "Query")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason for this change?

Copy link
Collaborator

@the-mayer the-mayer Oct 25, 2024

Choose a reason for hiding this comment

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

Ah, I see that aes_string() has been deprecated -- Instead, can you confirm that quoting the params of the aes() call still produces the desired result?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes @the-mayer, Its a Deprecation Adjustments:

It replaces aes_string() (deprecated) with aes() by quoting parameters, as suggested by a collaborator. This adjustment aligns the code with current ggplot2 standards while maintaining intended functionality.

) +
geom_tile(
data = subset(
all_grouped_reduced,
!is.na(count)
),
aes(fill = count),
colour = "darkred", size = 0.3
colour = "darkred", linewidth = 0.3
) + # , width=0.7, height=0.7),
scale_fill_gradient(low = "white", high = "darkred") +
# scale_x_discrete(position="top") +
Expand Down Expand Up @@ -1350,4 +1351,4 @@ plotLineageSunburst <- function(prot, lineage_column = "Lineage",
# # theme(axis.text.x=element_text(angle=90,hjust=1,vjust=0.5),
# # axis.text.y=element_text(angle=90,hjust=1,vjust=0.5))
#
# }
# }
Loading