Skip to content

Fix frollapply to handle n > len case #7650

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.

Sign up for GitHub

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

Open
jangorecki wants to merge 3 commits into
base: master
Choose a base branch
from
+9 −1
Open

Fix frollapply to handle n > len case #7650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0874a2d
Fix frollapply to handle n > len case
jangorecki Feb 26, 2026
4a537ee
unit test for bugfix
jangorecki Feb 26, 2026
281ab78
news entry
jangorecki Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@

5. Non-equi joins combining an equality condition with two inequality conditions on the same column (e.g., `on = .(id == id, val >= lo, val <= hi)`) no longer error, [#7641](https://github.com/Rdatatable/data.table/issues/7641). The internal `chmatchdup` remapping of duplicate `rightcols` was overwriting the original column indices, causing downstream code to reference non-existent columns. Thanks @tarun-t for the report and fix, and @aitap for the diagnosis.

6. `frollapply()` could have produce output longer than the input when the window-length is also longer than the input [#7646](https://github.com/Rdatatable/data.table/issues/7646). Thanks to @hadley-johnson for reporting and @jangorecki for the fix.
Copy link
Member

@MichaelChirico MichaelChirico Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
6. `frollapply()` could have produce output longer than the input when the window-length is also longer than the input [#7646](https://github.com/Rdatatable/data.table/issues/7646). Thanks to @hadley-johnson for reporting and @jangorecki for the fix.
6. `frollapply()` no longer produces output longer than the input when the window length is also longer than the input [#7646](https://github.com/Rdatatable/data.table/issues/7646). Thanks to @hadley-johnson for reporting and @jangorecki for the fix.


### Notes

1. {data.table} now depends on R 3.5.0 (2018).
Expand Down
5 changes: 4 additions & 1 deletion R/frollapply.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,11 @@ frollapply = function(X, N, FUN, ..., by.column=TRUE, fill=NA, align=c("right","
cpy = copy
ansMask = function(len, n) {
mask = rep(TRUE, len)
if (n) ## handle n==0
if (n) { ## handle n==0
if (n > len)
n = len + 1L ## bugfix #7646
mask[seq_len(n-1L)] = FALSE
}
mask
}
tight0 = function(i, dest, src, n) FUN(dest, ...) ## skip memcpy when n==0
Expand Down
3 changes: 3 additions & 0 deletions inst/tests/froll.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2317,3 +2317,6 @@ test(6015.907, frolladapt(c(1L,3L,2L), 2L), error="be sorted, have no duplicates
test(6015.908, frolladapt(c(1L,2L,2L), 2L), error="be sorted, have no duplicates, have no NAs")
test(6015.909, frolladapt(c(1L,2L,NA_integer_), 2L), error="be sorted, have no duplicates, have no NAs") ## loop that checks for sorted will detect NAs as well, except for first element
test(6015.910, frolladapt(c(NA_integer_,1L,2L), 2L), error="be sorted, have no duplicates, have no NAs") ## first NA is detected by extra check

# frollapply can produce output longer than the input when the window-length is also longer than the input #7646
test(6100.1, frollapply(c(1,2,3,4,5),N=7,FUN=sum), rep(NA, 5L))
Loading