Skip to content
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

Rewrite between() with vctrs #6260

Merged
merged 4 commits into from
May 9, 2022

Conversation

DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented May 6, 2022

Closes #6183

  • Supports more input types
  • No longer requires scalar left/right
  • Bounds are cast to type of x
  • Bounds are recycled to size of x

See also my funs PR where I discuss the enhancements I made there and talk a little about performance tidyverse/funs#78. This is basically a port of that.


Revdep run shows 6 failures that seem to be related to between() getting a little stricter (related to the fact that left and right are cast to the type of x, so you can't do between(1L, -Inf, Inf), for example). It says 9 failures but 3 are related to a failing internet resource. This seems reasonable.

Might need to adjust the NEWS bullet to talk more about the strictness.

@DavisVaughan DavisVaughan requested a review from hadley May 6, 2022 21:21
- Supports more input types
- No longer requires scalar left/right
- Bounds are cast to type of `x`
- Bounds are recycled to size of `x`
@DavisVaughan DavisVaughan force-pushed the feature/vctrs-between branch from b3a3dc0 to ed578c7 Compare May 9, 2022 19:35
Since it seems like the cause of most of the revdep failures
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.

between function bug for datetime/date mix
2 participants