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: add assert_named_list() helper to assert the value is a named list with no duplicate names #1322

Merged
merged 1 commit into from Mar 26, 2024

Conversation

maelle
Copy link
Contributor

@maelle maelle commented Mar 26, 2024

Fix #1321

Copy link
Contributor

aviator-app bot commented Mar 26, 2024

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@maelle maelle added the upkeep maintenance, infrastructure, and similar label Mar 26, 2024
@maelle maelle requested a review from krlmlr March 26, 2024 09:24
@@ -349,10 +349,7 @@ graph.attributes <- function(graph) {
#' @export
"graph.attributes<-" <- function(graph, value) {
ensure_igraph(graph)
if (!is.list(value) || (length(value) > 0 && is.null(names(value))) ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see why value would have no length, but I added the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well it does happen according to what happened in tests before I accounted for the case

@maelle maelle marked this pull request as draft March 26, 2024 09:51
@maelle maelle removed the request for review from krlmlr March 26, 2024 09:51
@maelle maelle marked this pull request as ready for review March 26, 2024 10:15
@maelle maelle requested review from krlmlr and removed request for krlmlr March 26, 2024 10:16
@maelle
Copy link
Contributor Author

maelle commented Mar 26, 2024

I'll go ahead and merge to continue my reading/refactoring journey of this script (before I can tackle #1303!)

@aviator-app aviator-app bot merged commit 7bebe49 into main Mar 26, 2024
14 checks passed
@aviator-app aviator-app bot deleted the assertlist branch March 26, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergequeue upkeep maintenance, infrastructure, and similar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor value assertion in R/attributes.R
1 participant