Skip to content

Conversation

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Jun 30, 2024

Recent duckplyr checks that all functions used in a dplyr verb are what they pretend they are: tidyverse/duckplyr#179. This works for most cases, except for the n() created by tally_n() . This PR fixes this.

Once we agree on the implementation, I can vendor in a copy of this function into duckplyr.

@krlmlr krlmlr requested a review from DavisVaughan June 30, 2024 16:52
@DavisVaughan
Copy link
Member

@krlmlr do you still use this feature in duckplyr? Would switching to dplyr::n() and base::sum() work instead?

@krlmlr
Copy link
Member Author

krlmlr commented Nov 17, 2025

dplyr::n() and base::sum() would be fine too, yes.

@DavisVaughan
Copy link
Member

So to be super clear, instead of expr(n()) we could put expr(dplyr::n()), and instead of expr(sum(!!wt, na.rm = TRUE)) we could put expr(base::sum(!!wt, na.rm = TRUE)), and that'd please duckplyr? That sounds much preferable to me!

@krlmlr

This comment was marked as outdated.

This comment was marked as outdated.

@krlmlr
Copy link
Member Author

krlmlr commented Nov 17, 2025

Should be good now.

@DavisVaughan DavisVaughan merged commit f18fac5 into main Nov 18, 2025
14 checks passed
@DavisVaughan DavisVaughan deleted the b-tally-quosure branch November 18, 2025 14:34
@DavisVaughan
Copy link
Member

Great thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants