Skip to content

refactor: convert magrittr %>% to base |> and ensure RHS calls use pa… #159

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.

Sign up for GitHub

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

Open
stemangiola wants to merge 18 commits into
base: master
Choose a base branch
from
Open

refactor: convert magrittr %>% to base |> and ensure RHS calls use pa… #159

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7e58cb6
refactor: convert magrittr %>% to base |> and ensure RHS calls use pa…
stemangiola Oct 25, 2025
77d148c
fix the pipe conversion
stemangiola Oct 25, 2025
dd21e31
Convert magrittr pipes to base R pipes
stemangiola Oct 25, 2025
329d8ba
Fix annotation_group examples to prevent CI/CD failures
stemangiola Oct 25, 2025
b8a85f1
Fix discarded pipe result in utilities.R
stemangiola Oct 25, 2025
dec654e
Refactor color mapping and data filtering logic
cursoragent Oct 25, 2025
35fa91b
Fix remaining when() calls and break pipe chains with if statements
stemangiola Oct 25, 2025
84cd62c
Fix all remaining when() calls and break pipe chains
stemangiola Oct 25, 2025
dcf750c
Use case_when() for cleaner conditional logic
stemangiola Oct 25, 2025
35a95ca
Fix as_matrix function to work with base R pipes
stemangiola Oct 25, 2025
ecd283d
Fix validation function parameter names for base R pipes
stemangiola Oct 25, 2025
02ced51
Fix logical condition in subset function for column validation
stemangiola Oct 25, 2025
4f65dc4
Fix %in% operator precedence issues with base R pipes
stemangiola Oct 25, 2025
41829e8
refactor: drop ifelse_pipe/ifelse2_pipe; simplify conditionals; fix t…
stemangiola Oct 28, 2025
ae5ba5b
Bump version from 1.12.3 to 1.12.4
stemangiola Oct 28, 2025
d4bb468
Update DESCRIPTION
stemangiola Oct 28, 2025
632259b
Merge branch 'master' into chore/convert-to-base-pipe
stemangiola Oct 28, 2025
6115d9a
fix: vignette build by normalizing annotation arg merging and list ha…
stemangiola Oct 28, 2025
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
210 changes: 105 additions & 105 deletions R/deprecated_framework.R
View file
Open in desktop

Large diffs are not rendered by default.

120 changes: 60 additions & 60 deletions R/functions.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#'
#' @import dplyr
#' @import tidyr
#' @importFrom magrittr "%>%"
#' @importFrom magrittr "|>"
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

The importFrom directive references |> from magrittr, but |> is a base R pipe operator (available in R >= 4.1.0), not from the magrittr package. This line should be removed.

Suggested change
#' @importFrom magrittr "|>"

Copilot uses AI. Check for mistakes.
#' @importFrom rlang enquo
#' @importFrom rlang quo_name
#' @importFrom circlize colorRamp2
Expand Down Expand Up @@ -70,75 +70,75 @@ input_heatmap = function(.data,

# Arguments
arguments =
as.list(environment()) %>%
as.list(environment()) |>
c(list(.horizontal = .horizontal, .vertical = .vertical, .abundance = .abundance))

# Check if palette discrete and continuous are lists
if(!is.list(palette_grouping) )
stop("tidyHeatmap says: the arguments palette_discrete and palette_continuous must be lists. E.g., list(rep(\"#000000\", 20))")

# Check that there have at least one value in the heatmap
if(.data %>% filter(!!.abundance %>% is.na %>% not %>% as.logical) %>% nrow %>% equals(0))
if(.data |> filter(!!.abundance |> is.na |> not |> as.logical) |> nrow()() |> equals(0))
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses nrow()() indicate incorrect syntax. The function should be called as nrow() after the pipe operator.

Copilot uses AI. Check for mistakes.
stop("tidyHeatmap says: your dataset does not have any non NA values")

# Get abundance matrix
abundance_tbl =
.data %>%
ungroup() %>%
.data |>
ungroup() |>

# Check if transform is needed
when(
is_function(transform) ~
mutate(., !!.abundance := !!.abundance %>% transform()) %>%
mutate(., !!.abundance := !!.abundance |> transform()) |>

# Check if log introduced -Inf
when(

# NAN produced
filter(., !!.abundance %>% is.nan %>% as.logical) %>% nrow %>% gt(0) ~ stop("tidyHeatmap says: you applied a transformation that introduced NaN."),
filter(., !!.abundance |> is.nan |> as.logical) |> nrow()() |> gt(0) ~ stop("tidyHeatmap says: you applied a transformation that introduced NaN."),
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses nrow()() indicate incorrect syntax. The function should be called as nrow() after the pipe operator.

Copilot uses AI. Check for mistakes.

# -Inf produced
pull(., !!.abundance) %>% min %>% equals(-Inf) ~ stop("tidyHeatmap says: you applied a transformation that introduced negative infinite .value, was it log? If so please use log1p."),
pull(., !!.abundance) |> min |> equals(-Inf) ~ stop("tidyHeatmap says: you applied a transformation that introduced negative infinite .value, was it log? If so please use log1p."),
~(.)
),
~ (.)
) %>%
) |>

# If scale row
when(
scale %in% c("row", "both") ~ (.) %>%
nest(data = -!!.vertical) %>%
mutate(data = map(data, ~ .x %>% mutate(!!.abundance := !!.abundance %>% scale_robust()))) %>%
scale %in% c("row", "both") ~ (.) |>
nest(data = -!!.vertical) |>
mutate(data = map(data, ~ .x |> mutate(!!.abundance := !!.abundance |> scale_robust()))) |>
unnest(data),
~ (.)
) %>%
) |>

# If scale column
when(
scale %in% c("column", "both") ~ (.) %>%
nest(data = -!!.horizontal) %>%
mutate(data = map(data, ~ .x %>% mutate(!!.abundance := !!.abundance %>% scale_robust()))) %>%
scale %in% c("column", "both") ~ (.) |>
nest(data = -!!.horizontal) |>
mutate(data = map(data, ~ .x |> mutate(!!.abundance := !!.abundance |> scale_robust()))) |>
unnest(data),
~ (.)
) %>%
) |>

distinct(!!.vertical,!!.horizontal,!!.abundance) %>%
distinct(!!.vertical,!!.horizontal,!!.abundance) |>

# Arrange both columns and rows
# do not leave the order of appearence dictate the order of columns and rows
pivot_wider(names_from = !!.horizontal, values_from = !!.abundance, names_sort = TRUE) |>
arrange(!!.vertical)

abundance_mat =
abundance_tbl %>%
abundance_tbl |>
as_matrix(rownames = quo_name(.vertical))

# Colors tiles
# If palette_value is a function pass it directly, otherwise check if the character array is of length 3
colors =
palette_value %>%
palette_value |>
when(
palette_value %>% class() %>% equals("function") ~ (.),
palette_value |> class()() |> equals("function") ~ (.),
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses class()() indicate incorrect syntax. The class() function should be called with single parentheses after the pipe operator.

Suggested change
palette_value |> class()() |> equals("function") ~ (.),
palette_value |> class() |> equals("function") ~ (.),

Copilot uses AI. Check for mistakes.
length(palette_value) != 3 ~ stop("tidyHeatmap says: If palette_value is a vector of hexadecimal colours, it should have 3 values. If you want more customisation, you can pass to palette_value a function, that is derived as for example \"colorRamp2(c(-2, 0, 2), palette_value)\"" ),

# For the crazy scenario when only one value is present in the heatmap (tidyHeatmap/issues/40)
Expand All @@ -161,7 +161,7 @@ input_heatmap = function(.data,
# Define object
new(
"InputHeatmap",
data = .data %>% reduce_to_tbl_if_in_class_chain,
data = .data |> reduce_to_tbl_if_in_class_chain,
# Due to the `.homonyms="last"` parameter, additional arguments passed by the user
# via `...` overwrite the defaults given below (See also `?rlang::dots_list`)
input = rlang::dots_list(
Expand All @@ -188,7 +188,7 @@ add_grouping = function(my_input_heatmap){


# Check if there are nested column in the data frame
if(my_input_heatmap@data %>% lapply(class) %>% equals("list") %>% any)
if(my_input_heatmap@data |> lapply(class) |> equals("list") |> any)
warning("tidyHeatmap says: nested/list column are present in your data frame and have been dropped as their unicity cannot be identified by dplyr.")

# Column names
Expand All @@ -197,35 +197,35 @@ add_grouping = function(my_input_heatmap){
.abundance = my_input_heatmap@arguments$.abundance

# Number of groups
how_many_groups = my_input_heatmap@data %>% attr("groups") %>% nrow
how_many_groups = my_input_heatmap@data |> attr("groups") |> nrow()()
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses nrow()() indicate incorrect syntax. The function should be called as nrow() after the pipe operator.

Suggested change
how_many_groups = my_input_heatmap@data |> attr("groups") |> nrow()()
how_many_groups = my_input_heatmap@data |> attr("groups") |> nrow()

Copilot uses AI. Check for mistakes.

# Number of grouping
how_many_grouping = my_input_heatmap@data %>% attr("groups") %>% select(-.rows) %>% ncol
how_many_grouping = my_input_heatmap@data |> attr("groups") |> select(-.rows) |> ncol()()
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses ncol()() indicate incorrect syntax. The function should be called as ncol() after the pipe operator.

Suggested change
how_many_grouping = my_input_heatmap@data |> attr("groups") |> select(-.rows) |> ncol()()
how_many_grouping = my_input_heatmap@data |> attr("groups") |> select(-.rows) |> ncol()

Copilot uses AI. Check for mistakes.

# Add custom palette to discrete if any
my_input_heatmap@palette_discrete =
my_input_heatmap@arguments$palette_grouping %>%
my_input_heatmap@arguments$palette_grouping |>
when(
length(.) < how_many_grouping ~ {
# Needed for piping
pg = .

my_input_heatmap@arguments$palette_grouping %>%
my_input_heatmap@arguments$palette_grouping |>
c(
rep("#ffffff", how_many_groups) %>%
list() %>%
rep("#ffffff", how_many_groups) |>
list() |>
rep(how_many_grouping-length(pg))
)
},
~ (.)
) %>%
) |>
c(my_input_heatmap@palette_discrete)

# Colours annotations
palette_annotation = my_input_heatmap@palette_discrete %>% head(how_many_grouping)
palette_annotation = my_input_heatmap@palette_discrete |> head(how_many_grouping)

# Take away used palettes
my_input_heatmap@palette_discrete = my_input_heatmap@palette_discrete %>% tail(-how_many_grouping)
my_input_heatmap@palette_discrete = my_input_heatmap@palette_discrete |> tail(-how_many_grouping)

# See if I have grouping and setup framework
group_annotation =
Expand All @@ -247,11 +247,11 @@ add_grouping = function(my_input_heatmap){
my_input_heatmap@group_left_annotation = group_annotation$left_annotation

my_input_heatmap@input =
my_input_heatmap@input %>%
my_input_heatmap@input |>
when(
!is.null(group_annotation$row_split) ~ c(., list(row_split = group_annotation$row_split, cluster_row_slices = FALSE)),
~ (.)
) %>%
) |>
when(
!is.null(group_annotation$col_split) ~ c(., list(column_split = group_annotation$col_split, cluster_column_slices = FALSE)),
~ (.)
Expand All @@ -267,7 +267,7 @@ add_grouping = function(my_input_heatmap){
#'
#' @import dplyr
#' @import tidyr
#' @importFrom magrittr "%>%"
#' @importFrom magrittr "|>"
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

The importFrom directive references |> from magrittr, but |> is a base R pipe operator (available in R >= 4.1.0), not from the magrittr package. This line should be removed.

Suggested change
#' @importFrom magrittr "|>"

Copilot uses AI. Check for mistakes.
#' @importFrom rlang enquo
#' @importFrom rlang quo_name
#' @importFrom circlize colorRamp2
Expand Down Expand Up @@ -336,8 +336,8 @@ add_annotation = function(my_input_heatmap,
stop("tidyHeatmap says: the arguments palette_discrete and palette_continuous must be lists. E.g., list(rep(\"#000000\", 20))")

# Add custom palette to discrete if any
my_input_heatmap@palette_discrete = palette_discrete %>% c(my_input_heatmap@palette_discrete)
my_input_heatmap@palette_continuous = palette_continuous %>% c(my_input_heatmap@palette_continuous)
my_input_heatmap@palette_discrete = palette_discrete |> c(my_input_heatmap@palette_discrete)
my_input_heatmap@palette_continuous = palette_continuous |> c(my_input_heatmap@palette_continuous)

# Colors annotations
palette_annotation = list(
Expand All @@ -346,64 +346,64 @@ add_annotation = function(my_input_heatmap,
)

# Check if there are nested column in the data frame
if(.data %>% lapply(class) %>% equals("list") %>% any)
if(.data |> lapply(class) |> equals("list") |> any)
warning("tidyHeatmap says: nested/list column are present in your data frame and have been dropped as their unicity cannot be identified by dplyr.")

# Data frame of row and column columns
x_y_annot_cols =
.data %>%
.data |>
get_x_y_annotation_columns(!!.horizontal,!!.vertical,!!.abundance)

# Check if annotation is compatible with your dataset
quo_names(annotation) %>%
setdiff(x_y_annot_cols %>% pull(col_name)) %>%
quo_names(annotation) |>
setdiff(x_y_annot_cols |> pull(col_name)) |>
when( quo_names(annotation) != "NULL" & length(.) > 0 ~
stop(
sprintf(
"tidyHeatmap says: Your annotation \"%s\" is not unique to vertical nor horizontal dimentions",
(.) %>% paste(collapse = ", ")
(.) |> paste(collapse = ", ")
)
))

# Get annotation
.data_annot =
.data %>%
.data |>
get_top_left_annotation( !!.horizontal,
!!.vertical,
!!.abundance,
!!annotation, palette_annotation, type, x_y_annot_cols, size, ...)

# Number of grouping
how_many_discrete = .data_annot %>% filter(annot_type=="discrete") %>% nrow
how_many_continuous = .data_annot %>% filter(annot_type=="continuous") %>% nrow
how_many_discrete = .data_annot |> filter(annot_type=="discrete") |> nrow()()
how_many_continuous = .data_annot |> filter(annot_type=="continuous") |> nrow()()
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses nrow()() indicate incorrect syntax. The function should be called as nrow() after the pipe operator.

Suggested change
how_many_discrete = .data_annot |> filter(annot_type=="discrete") |> nrow()()
how_many_continuous = .data_annot |> filter(annot_type=="continuous") |> nrow()()
how_many_discrete = .data_annot |> filter(annot_type=="discrete") |> nrow()
how_many_continuous = .data_annot |> filter(annot_type=="continuous") |> nrow()

Copilot uses AI. Check for mistakes.

# Eliminate used annotations
my_input_heatmap@palette_discrete = my_input_heatmap@palette_discrete %>% when(how_many_discrete>0 ~ tail(., -how_many_discrete) , ~ (.))
my_input_heatmap@palette_continuous = my_input_heatmap@palette_continuous %>% when(how_many_continuous>0 ~ tail(., -how_many_continuous), ~ (.))
my_input_heatmap@palette_discrete = my_input_heatmap@palette_discrete |> when(how_many_discrete>0 ~ tail(., -how_many_discrete) , ~ (.))
my_input_heatmap@palette_continuous = my_input_heatmap@palette_continuous |> when(how_many_continuous>0 ~ tail(., -how_many_continuous), ~ (.))

# # Check if annotation is compatible with your dataset
# x_y_annot_cols %>%
# inner_join(.data_annot %>% distinct(col_name), by="col_name") %>%
# count(col_name) %>%
# filter(n > 1) %>%
# pull(col_name) %>%
# x_y_annot_cols |>
# inner_join(.data_annot |> distinct(col_name), by="col_name") |>
# count(col_name) |>
# filter(n > 1) |>
# pull(col_name) |>
# when( length(.) > 0 ~
# stop(
# sprintf(
# "tidyHeatmap says: Your annotation \"%s\" is unique to vertical and horizontal dimentions",
# (.) %>% paste(collapse = ", ")
# (.) |> paste(collapse = ", ")
# )
# ))

# Isolate top annotation
my_input_heatmap@top_annotation =
my_input_heatmap@top_annotation %>%
bind_rows(.data_annot %>% filter(orientation == "column") )
my_input_heatmap@top_annotation |>
bind_rows(.data_annot |> filter(orientation == "column") )

# Isolate left annotation
my_input_heatmap@left_annotation =
my_input_heatmap@left_annotation %>%
bind_rows( .data_annot %>% filter(orientation == "row") )
my_input_heatmap@left_annotation |>
bind_rows( .data_annot |> filter(orientation == "row") )

my_input_heatmap

Expand All @@ -416,7 +416,7 @@ add_annotation = function(my_input_heatmap,
#' @description layer_symbol() from a `InputHeatmap` object, adds a symbol annotation layer.
#'
#' @importFrom rlang enquo
#' @importFrom magrittr "%>%"
#' @importFrom magrittr "|>"
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

The importFrom directive references |> from magrittr, but |> is a base R pipe operator (available in R >= 4.1.0), not from the magrittr package. This line should be removed.

Suggested change
#' @importFrom magrittr "|>"

Copilot uses AI. Check for mistakes.
#'
#'
#'
Expand Down Expand Up @@ -514,8 +514,8 @@ setMethod("layer_symbol", "InputHeatmap", function(.data,
.data_drame |>
droplevels() |>
mutate(
column = !!.horizontal %>% as.factor() %>% as.integer(),
row = !!.vertical %>% as.factor() %>% as.integer()
column = !!.horizontal |> as.factor() |> as.integer(),
row = !!.vertical |> as.factor() |> as.integer()
) |>
filter(...) |>
mutate(shape = symbol_dictionary[[!!symbol]]) |>
Expand Down
8 changes: 4 additions & 4 deletions R/methods.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ heatmap_ <-
) |>

# Add group annotation if any
when( "groups" %in% (attributes(.data) |> names()) ~ add_grouping(.), ~ (.))
when( "groups" %in% (attributes(.data) |> names()()) ~ add_grouping(.), ~ (.))
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses names()() indicate incorrect syntax. The names() function should be called with single parentheses after the pipe operator.

Suggested change
when( "groups" %in% (attributes(.data) |> names()()) ~ add_grouping(.), ~ (.))
when( "groups" %in% (attributes(.data) |> names()) ~ add_grouping(.), ~ (.))

Copilot uses AI. Check for mistakes.

}

Expand Down Expand Up @@ -1139,7 +1139,7 @@ setMethod("layer_asterisk", "InputHeatmap", function(.data,...,
#' @description layer_text() from a `InputHeatmap` object, adds a text annotation layer.
#'
#' @importFrom rlang enquo
#' @importFrom magrittr "%>%"
#' @importFrom magrittr "|>"
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

The importFrom directive references |> from magrittr, but |> is a base R pipe operator (available in R >= 4.1.0), not from the magrittr package. This line should be removed.

Suggested change
#' @importFrom magrittr "|>"

Copilot uses AI. Check for mistakes.
#'
#'
#'
Expand Down Expand Up @@ -1224,8 +1224,8 @@ setMethod("layer_text", "InputHeatmap", function(.data,
.data_drame |>
droplevels() |>
mutate(
column = !!.horizontal %>% as.factor() %>% as.integer(),
row = !!.vertical %>% as.factor() %>% as.integer()
column = !!.horizontal |> as.factor() |> as.integer(),
row = !!.vertical |> as.factor() |> as.integer()
) |>
filter(...) |>
mutate(text := as.character( !!enquo(.value) )) |>
Expand Down
26 changes: 13 additions & 13 deletions R/subset.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,28 @@ get_specific_annotation_columns = function(.data, .col){
.col = enquo(.col)

# x-annotation df
n_x = .data %>% select(!!.col) |> distinct() %>% nrow
n_x = .data |> select(!!.col) |> distinct() |> nrow()()
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Double parentheses nrow()() indicate incorrect syntax. The function should be called as nrow() after the pipe operator.

Suggested change
n_x = .data |> select(!!.col) |> distinct() |> nrow()()
n_x = .data |> select(!!.col) |> distinct() |> nrow()

Copilot uses AI. Check for mistakes.

# element wise columns
.data %>%
select(-!!.col) %>%
colnames %>%
.data |>
select(-!!.col) |>
colnames |>
map(
~
.x %>%
.x |>
ifelse_pipe(
.data %>%
.data |>
select(!!.col, all_of(.x)) |>
distinct() %>%
nrow %>%
distinct() |>
nrow |>
equals(n_x),
~ .x,
~ NULL
)
) %>%
) |>

# Drop NULL
{ (.)[lengths((.)) != 0] } %>%
{ (.)[lengths((.)) != 0] } |>
unlist

}
Expand All @@ -43,13 +43,13 @@ subset = function(.data,
.column = enquo(.column)

# Check if column present
if(quo_names(.column) %in% colnames(.data) %>% all %>% `!`)
if(quo_names(.column) %in% colnames(.data) |> all |> `!`)
stop("nanny says: some of the .column specified do not exist in the input data frame.")

.data %>%
.data |>

# Selecting the right columns
select( !!.column, all_of(get_specific_annotation_columns(.data, !!.column) )) %>%
select( !!.column, all_of(get_specific_annotation_columns(.data, !!.column) )) |>
distinct()

}
Expand Down
Loading
Loading