Skip to content

Fix vec_ptype2() fallback path when LHS is NULL#2128

Open
DavisVaughan wants to merge 4 commits intomainfrom
fix/ptype2-fallback-typo
Open

Fix vec_ptype2() fallback path when LHS is NULL#2128
DavisVaughan wants to merge 4 commits intomainfrom
fix/ptype2-fallback-typo

Conversation

@DavisVaughan
Copy link
Copy Markdown
Member

@DavisVaughan DavisVaughan commented Dec 17, 2025

Closes #2127

The goal of this PR was to fix this small x_type -> y_type typo here

return vec_ptype_or_s3_fallback(y, p_y_arg, x_type, call, s3_fallback);

But since that is run as the very first iteration of almost every vec_ptype_common() call (because we start the reduction with x = NULL, y = xs[[1]]), it had a huge amount of fallout of weird buggy cases that relied on this.

@DavisVaughan DavisVaughan requested a review from lionel- December 17, 2025 21:10
@DavisVaughan
Copy link
Copy Markdown
Member Author

DavisVaughan commented Dec 19, 2025

There are unfortunately more revdep failures we are going to have to deal with from this

Some related to POSIXlt going towards POSIXct more often. Some related to ptype2 being called more often, I think.

@DavisVaughan DavisVaughan force-pushed the fix/ptype2-fallback-typo branch 2 times, most recently from f0074cc to 73581e7 Compare January 2, 2026 20:30
@DavisVaughan
Copy link
Copy Markdown
Member Author

DavisVaughan commented Jan 5, 2026

We won't be able to do this right now due to the complexity of getting the sf methods compatible with this change, so we are going to have to wait on most of this until we can move all of the vctrs methods into sf itself first
r-spatial/sf#2568

After that, it should hopefully be a bit easier to do this, since all of the vctrs methods will live in one place.

Ideally, sf won't need to use the vctrs fallback mechanism at all, allowing us to simplify things as we've tried to do here (i.e. by removing vec_ptype_or_s3_fallback() in favor of just vec_ptype(), avoiding the weird ptype2 computation there)


I'll try and pluck out some of the unrelated changes (POSIXlt methods, any extra tests, etc) into their own PR to get those in.

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.

Inconsistent type passed through in fallback path of vec_ptype2()

2 participants