-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add prettify to validate_indices #59
Conversation
R/accessory.R
Outdated
@@ -1271,6 +1271,7 @@ validate_indices <- function(assay_data, index_string) { | |||
indices <- as.integer(simpleSplit(index_string)) | |||
} else { | |||
indices <- simpleSplit(index_string) | |||
indices <- lapply(indices, prettifyVariablename) |
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.
You'll need an unlist here to maintain the same type as simpleSplit.
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.
Sorry, just looking at this properly.
You'll notice that stringsToNamedVector has the prettification as a parameter. It should probably be the same in this function (with the same default), so that things can be matched.
Ah yes sure, done! |
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.
LGTM
ATM, a call to
prettifyVariablename
is missing invalidate_indices
whilestringsToNamedVector
contains a prettify. This leads toexploratory_plots.R
throwing anerror Invalid
assays because it will e.g. compare the stringsraw
andRaw
. This PR adds the prettify to fix this issue.