search

dexter-psychometrics / dexter

Management, assessment, and psychometric analysis of data from educational and psychological tests
GNU Lesser General Public License v3.0
8 stars 5 forks source link

Call `arrange()` after `group_by() + summarize()` #6

Closed DavisVaughan closed 2 years ago

DavisVaughan commented 2 years ago

Hi there, we ran revdeps for dplyr and your package appeared in our checks.

We have updated arrange() to now have an explicit .locale argument that defaults to American English. It previously respected the system locale, but this is more performant and reproducible across R sessions.

You currently rely on the resulting ordering of group_by(df, x) %>% summarize() matching the ordering you'd get back from arrange(df, x). Instead, we suggest explicitly calling arrange() after calling summarize() if you rely on that ordering, as it will no longer match arrange() automatically in the next version of dplyr.

If you install this PR, you will see that one of your tests fails because of this assumption. https://github.com/tidyverse/dplyr/pull/6263

Luckily, the fix here is simple and work with both dev and CRAN dplyr! If you could merge this PR and then go ahead and send a release to CRAN, then we would greatly appreciate it. It will make releasing the next version of dplyr easier for us.

Thanks!

jessekps commented 2 years ago

Great catch, tnx. I certainly did not intend to rely on group_by's ordering. I'll have to check if it occurs anywhere else. I had planned the next cran release for end of next week and will include this fix

jessekps commented 2 years ago

the fix is currently on its way to cran

DavisVaughan commented 2 years ago

Thank you!