Uh oh!
There was an error while loading. Please reload this page.
ARROW-13739: [R] Support dplyr::count() and tally() - #11306
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
nealrichardson
commented
Oct 4, 2021
There's also a skipped test-dataset-something.R test about count, you should unskip it in this PR |
Uh oh!
There was an error while loading. Please reload this page.
The failing test here fails once per session with something like but on subsequent runs in the same session, it's fine and passes. I'm a bit unsure of what to do here as I looked for the line of code which triggers it but see no Is this something we've seen before @nealrichardson ? |
3c2899f to
b551527CompareUh oh!
There was an error while loading. Please reload this page.
nealrichardson
commented
Oct 6, 2021
"This warning is displayed once per session" is why it doesn't happen the next time. I've seen it before but not lately. I'll have to pull and take a look. |
Uh oh!
There was an error while loading. Please reload this page.
nealrichardson
commented
Oct 6, 2021
Turns out we're getting quosures inside of quosures still. I'll suggest another patch, which might help for any others who are building NSE wrappers, and if that doesn't work, we can go back to the |
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
2470920 to
23dd3dfComparenealrichardson
commented
Oct 6, 2021
I've fixed the tests, 🤞 we get a passing build now |
| is_function <- function(expr, name) { | ||
| # We could have a quosure here if we have an expression like | ||
| # `sum({{ var }})` |
There was a problem hiding this comment.
@jonkeane How do I make this lint message go away?
There was a problem hiding this comment.
There are a few options: having more words in front of the code or # nolint at the end. We can also turn this one particular lintr off (or it's possible there are settings for it that makes it less sensitive) by adding commented_code_linter = NULL to https://github.com/apache/arrow/blob/master/r/.lintr#L25
I've been meaning to send a PR to lintr to make backticks like this defuse commented code — though their track record of merging these has been spotty so I haven't made it a priority.
Uh oh!
There was an error while loading. Please reload this page.
nealrichardson
left a comment
There was a problem hiding this comment.
Just need to fix the style/lint stuff but then LGTM!
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.
No description provided.