-
-
Notifications
You must be signed in to change notification settings - Fork 206
feat: Add attribute combination support to graph operators #2676
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
base: main
Are you sure you want to change the base?
Changes from all commits
c75e174
41c4b0c
5f7521a
cb49b91
d7e8bdc
c26ab02
f9ad5b3
b58da44
04ab715
6d94668
5bebc2e
05336ba
a85a78c
d1ac089
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1290,7 +1290,7 @@ is_bipartite <- function(graph) { | |
|
|
||
| ############# | ||
|
|
||
| igraph.i.attribute.combination <- function(comb) { | ||
| igraph.i.attribute.combination <- function(comb, allow_rename = FALSE) { | ||
| if (is.function(comb)) { | ||
| comb <- list(comb) | ||
| } | ||
|
|
@@ -1310,34 +1310,40 @@ igraph.i.attribute.combination <- function(comb) { | |
| if (anyDuplicated(names(comb)) > 0) { | ||
| cli::cli_warn("Some attributes are duplicated") | ||
| } | ||
| known_names <- c( | ||
| "ignore", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. order them alphabetically? |
||
| "sum", | ||
| "prod", | ||
| "min", | ||
| "max", | ||
| "random", | ||
| "first", | ||
| "last", | ||
| "mean", | ||
| "median", | ||
| "concat" | ||
| ) | ||
| known_codes <- c(0, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add an explanatory comment? Also, do we use explicit integers? And would there be a way to define each code beside its name? |
||
| if (allow_rename) { | ||
| known_names <- c(known_names, "rename") | ||
| known_codes <- c(known_codes, NA_integer_) | ||
| } | ||
| comb <- lapply(comb, function(x) { | ||
| if (!is.character(x)) { | ||
| x | ||
| } else { | ||
| known <- data.frame( | ||
| n = c( | ||
| "ignore", | ||
| "sum", | ||
| "prod", | ||
| "min", | ||
| "max", | ||
| "random", | ||
| "first", | ||
| "last", | ||
| "mean", | ||
| "median", | ||
| "concat" | ||
| ), | ||
| i = c(0, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12), | ||
| stringsAsFactors = FALSE | ||
| ) | ||
| x <- pmatch(tolower(x), known[, 1]) | ||
| if (is.na(x)) { | ||
| idx <- pmatch(tolower(x), known_names) | ||
| if (is.na(idx)) { | ||
| if (!allow_rename && identical(tolower(x), "rename")) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be more logical to me to invert the two if (identical(tolower(x), "rename") && !allow_rename) {
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why identical btw? |
||
| cli::cli_abort( | ||
| "{.val rename} is only supported by graph operators ({.fn union}, {.fn intersection}, {.fn compose}, {.fn disjoint_union}), not by this function." | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rephrase to
|
||
| ) | ||
| } | ||
| cli::cli_abort( | ||
| "Unknown/unambigous attribute combination specification." | ||
| ) | ||
| } | ||
| known[, 2][x] | ||
| if (is.na(known_codes[idx])) "rename" else known_codes[idx] | ||
| } | ||
| }) | ||
|
|
||
|
|
@@ -1353,7 +1359,7 @@ igraph.i.attribute.combination <- function(comb) { | |
| #' vertex/edge attributes in these cases. | ||
| #' | ||
| #' The functions that support the combination of attributes have one or two | ||
| #' extra arguments called `vertex.attr.comb` and/or `edge.attr.comb` | ||
| #' extra arguments called `vertex_attr_combine` and/or `edge_attr_combine` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeing a clear name makes me so happy 😭 🪮 |
||
| #' that specify how to perform the mapping of the attributes. E.g. | ||
| #' [contract()] contracts many vertices into a single one, the | ||
| #' attributes of the vertices can be combined and stores as the vertex | ||
|
|
@@ -1435,6 +1441,15 @@ igraph.i.attribute.combination <- function(comb) { | |
| #' Concatenate the attributes, using the [c()] function. | ||
| #' This results almost always a complex attribute. | ||
| #' } | ||
| #' \item{"rename"}{ | ||
| #' Keep clashing attributes side-by-side under disambiguated names by | ||
| #' appending `_1`, `_2`, ... suffixes. This is the default for the | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add an example à la "if two graphs have an attribute called |
||
| #' graph operators [union()], [intersection()], [compose()] and | ||
| #' [disjoint_union()] and preserves their historical behaviour. | ||
| #' Only those operators accept `"rename"`; [simplify()] and | ||
| #' [contract()] will reject it because the rename strategy has no | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this sentence could be clarified: "it's hard to know where each attribute came from" or so? |
||
| #' per-element interpretation when many input values collapse into one. | ||
| #' } | ||
| #' } | ||
| #' @author Gabor Csardi \email{csardi.gabor@@gmail.com} | ||
| #' @seealso [graph_attr()], [vertex_attr()], | ||
|
|
@@ -1452,22 +1467,22 @@ igraph.i.attribute.combination <- function(comb) { | |
| #' igraph_options(print.edge.attributes = TRUE) | ||
| #' | ||
| #' ## new attribute is the sum of the old ones | ||
| #' simplify(g, edge.attr.comb = "sum") | ||
| #' simplify(g, edge_attr_combine = "sum") | ||
| #' | ||
| #' ## collect attributes into a string | ||
| #' simplify(g, edge.attr.comb = toString) | ||
| #' simplify(g, edge_attr_combine = toString) | ||
| #' | ||
| #' ## concatenate them into a vector, this creates a complex | ||
| #' ## attribute | ||
| #' simplify(g, edge.attr.comb = "concat") | ||
| #' simplify(g, edge_attr_combine = "concat") | ||
| #' | ||
| #' E(g)$name <- letters[seq_len(ecount(g))] | ||
| #' | ||
| #' ## both attributes are collected into strings | ||
| #' simplify(g, edge.attr.comb = toString) | ||
| #' simplify(g, edge_attr_combine = toString) | ||
| #' | ||
| #' ## harmonic average of weights, names are dropped | ||
| #' simplify(g, edge.attr.comb = list( | ||
| #' simplify(g, edge_attr_combine = list( | ||
| #' weight = function(x) length(x) / sum(1 / x), | ||
| #' name = "ignore" | ||
| #' )) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split into two functions, one for a scalar, and the list function as a simple wrapper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not.