Skip to content

Fix inverse_rle with negative lengths #853

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

Merged
nalimilan merged 4 commits into from
Apr 26, 2023
Merged

Fix inverse_rle with negative lengths #853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
066728d
Fix `inverse_rle` with negative lengths
ararslan Feb 27, 2023
36da73f
Move check to the loop to avoid going over the input twice
ararslan Mar 29, 2023
b15e59c
Revert line removal
nalimilan Apr 24, 2023
74d3aad
Add a check on the sum of the lengths
ararslan Apr 25, 2023
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: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "StatsBase"
uuid = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
authors = ["JuliaStats"]
version = "0.33.21"
version = "0.33.22"

[deps]
DataAPI = "9a962f9c-6df0-11e9-0e5d-c546b8b5ee8a"
Expand Down
5 changes: 4 additions & 1 deletion src/misc.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ run lengths.
function inverse_rle(vals::AbstractVector{T}, lens::AbstractVector{<:Integer}) where T
m = length(vals)
length(lens) == m || raise_dimerror()
n = sum(lens)
n >= 0 || throw(ArgumentError("lengths must be non-negative"))

r = Vector{T}(undef, sum(lens))
r = Vector{T}(undef, n)
p = 0
Copy link
Member

@nalimilan nalimilan Apr 24, 2023

Choose a reason for hiding this comment

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

Damn, sum(lens) on the line above can be negative, which throws an ErrorException instead of the expected ArgumentError. Should we add a check like this?

n = sum(lens)
n >= 0 || throw(ArgumentError("lengths must be non-negative"))

Otherwise, let's go back to the previous version.

Copy link
Member Author

@ararslan ararslan Apr 25, 2023

Choose a reason for hiding this comment

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

Added a check on the sum per your suggestion since we need to compute the sum anyway in order to initialize the output array.

@inbounds for i = 1 : m
j = lens[i]
j >= 0 || throw(ArgumentError("lengths must be non-negative"))
v = vals[i]
while j > 0
r[p+=1] = v
Expand Down
1 change: 1 addition & 0 deletions test/misc.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ z = [1, 1, 2, 2, 2, 3, 1, 2, 2, 3, 3, 3, 3]
@test vals == [1, 2, 3, 1, 2, 3]
@test lens == [2, 3, 1, 1, 2, 4]
@test inverse_rle(vals, lens) == z
@test_throws ArgumentError inverse_rle(vals, fill(-1, length(lens)))

z = [true, true, false, false, true, false, true, true, true]
vals, lens = rle(z)
Expand Down