-
Notifications
You must be signed in to change notification settings - Fork 81
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
returning a list of nodesets #311
Comments
What about calling the argument xml_find_all.xml_nodeset <- function (x, xpath, ns = xml_ns(x), flatten = TRUE) {
if (length(x) == 0)
return(xml_nodeset())
res <- lapply(x, function(x) .Call(xpath_search, x$node, x$doc, xpath, ns, Inf))
if (isTRUE(flatten)) {
return(xml_nodeset(unlist(recursive = FALSE, res)))
}
res[] <- lapply(res, xml_nodeset)
res
} |
I like the minimal redundancy. I initially considered something similar, but didn't know if calling lapply twice could slow the function down. Now that I've actually tested it though, the timings are virtually identical, at least on my xml files. Looks like you have this under control. I'm sure you can add documentation and unit tests faster than I can, but let me know how I can help. |
If you want this feature please open a PR including the documentation and unit tests. |
Hello, I have a use case in which I want to run
xml_find_all
on a nodeset, but have the result be a list of nodesets (one element for each node) instead of one flattened nodeset. In this use case, it's important that I know which result node came from which original node. In my testing it's way faster to have this functionality within the xml2 package, since it can directly call C in the lapply, than to callxml_find_all
in a for or foreach loop. I can imagine a couple ways to do this.xml_find_all.xml_nodeset
method by adding anunlist
argument. This would break the convention that "xml_find_all
always returns a nodeset", but seems cleaner to me. Something like below:xml_find_all_list
, something like below:I'm happy to make a PR, just let me know how to proceed. Thanks.
The text was updated successfully, but these errors were encountered: