Skip to content

chore: Avoid aliases in Stimulus#722

Merged
krlmlr merged 5 commits intomainfrom
f-unname-2
Mar 30, 2023
Merged

chore: Avoid aliases in Stimulus#722
krlmlr merged 5 commits intomainfrom
f-unname-2

Conversation

@krlmlr
Copy link
Copy Markdown
Contributor

@krlmlr krlmlr commented Mar 18, 2023

We can now handle aliases directly in the R code, this removes one layer of indirection.

Implemented by the following script:

library(conflicted)
library(tidyverse)
conflicted::conflicts_prefer(purrr::set_names)

yaml <- yaml::read_yaml("tools/stimulus/functions-R.yaml")
yaml


# Update functions-R.yaml -------------------------------------------------------------

non_name_list <- map(yaml, ~ .x[names(.x) != "NAME"])
has_non_name <- lengths(non_name_list) != 0
name_list <- map(yaml, ~ .x[names(.x) == "NAME"])
has_name <- lengths(name_list) != 0
names <- unlist(map(yaml, ~ .x["NAME"][[1]]))

search <- set_names(paste0("NAME: ", rex::escape(str_replace(names, "^igraph_", "")), "$"), names(has_name[has_name]))

text <- brio::read_lines("tools/stimulus/functions-R.yaml")
matches <- map(search, ~ str_which(text, .x))

stopifnot(lengths(matches) == 1)
stopifnot(has_name | has_non_name)

delete_three <- names(has_non_name[!has_non_name])

matches[delete_three] <- map(matches[delete_three], ~ .x - 0:2)

new_text <- text[-unlist(matches)]
brio::write_lines(new_text, "tools/stimulus/functions-R.yaml")


# Replace in code ---------------------------------------------------------------------

names_sorted <- names[order(-nchar(names))]

search <- paste0(names_sorted, "_impl")
replace <- paste0(str_replace(names(names_sorted), "^igraph_", ""), "_impl")

replace_all <- function(x) {
  reduce2(search, replace, ~ str_replace_all(..1, fixed(..2), ..3), .init = x)
}

replace_all_in_file <- function(path) {
  text <- brio::read_file(path)
  new_text <- replace_all(text)
  brio::write_file(new_text, path)
}

walk(fs::dir_ls("R", glob = "*.R"), replace_all_in_file)

@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 18, 2023

@maelle: Will there be conflicts with your work? I can rerun my code very easily on top of your changes if needed.

@krlmlr krlmlr requested a review from maelle March 18, 2023 19:30
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 18, 2023

Codecov Report

Merging #722 (68f77eb) into main (2f39a9d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 68f77eb differs from pull request most recent head f7d6cdd. Consider uploading reports for the commit f7d6cdd to get more accurate results

@@            Coverage Diff             @@
##             main     #722      +/-   ##
==========================================
+ Coverage   53.92%   53.95%   +0.02%     
==========================================
  Files         357      355       -2     
  Lines       73281    73344      +63     
==========================================
+ Hits        39519    39574      +55     
- Misses      33762    33770       +8     
Impacted Files Coverage Δ
R/aaa-auto.R 76.44% <ø> (ø)
R/bipartite.R 85.71% <ø> (ø)
R/centrality.R 74.75% <ø> (ø)
R/centralization.R 82.60% <ø> (ø)
R/community.R 74.63% <ø> (ø)
R/conversion.R 73.91% <ø> (ø)
R/embedding.R 100.00% <ø> (ø)
R/flow.R 79.13% <ø> (ø)
R/hrg.R 34.32% <ø> (+0.21%) ⬆️
R/interface.R 83.58% <ø> (ø)
... and 8 more

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@krlmlr krlmlr added this to the autogen milestone Mar 18, 2023
@krlmlr krlmlr marked this pull request as draft March 18, 2023 20:27
@maelle
Copy link
Copy Markdown
Contributor

maelle commented Mar 20, 2023

Does this rename, or move the definition of, functions used in https://github.com/igraph/rigraph/blob/main/R/zzz-deprecate.R (second arguments of calls to deprecated())? If so, yes that would influence my script, thanks for asking.

@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 23, 2023

No, R/zzz.R is not touched. Can we merge it then?

@krlmlr krlmlr marked this pull request as ready for review March 23, 2023 14:59
@maelle
Copy link
Copy Markdown
Contributor

maelle commented Mar 27, 2023

we decided waiting for my finalizing #716

@krlmlr krlmlr merged commit f1dad55 into main Mar 30, 2023
@krlmlr krlmlr deleted the f-unname-2 branch March 30, 2023 16:11
@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Mar 30, 2023

Sorry, trying to avoid cascades of blockers here. We'll work around any problems that arise from that.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants