Skip to content

Commit

Permalink
Fix assertions in constructor (#80)
Browse files Browse the repository at this point in the history
  • Loading branch information
bkamins authored Mar 23, 2022
1 parent 53f57d4 commit ec427e8
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/PooledArrays.jl
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ PooledArray
end

# Assertions are needed since _label is not type stable
return PooledArray(RefArray(refs::Vector{R}), invpool::Dict{T,R}, pool)
return PooledArray(RefArray(refs::Array{R, ndims(d)}), invpool::Dict{T,R}, pool)
end

@inline function PooledArray{T}(d::AbstractArray; signed::Bool=false, compress::Bool=false) where {T}
Expand Down
3 changes: 3 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ end

for T in (Int8, Int16, Int32, Int64, UInt8, UInt16, UInt32, UInt64)
@inferred PooledArray([1, 2, 3], T)
@inferred PooledArray([1 2 3], T)
end
@test typeof(PooledArray([1 2; 3 4], Int8)) === PooledMatrix{Int, Int8, Matrix{Int8}}

for signed in (true, false), compress in (true, false)
@test_throws ErrorException @inferred PooledArray([1, 2, 3],
signed=signed,
Expand Down

4 comments on commit ec427e8

@iamed2
Copy link

@iamed2 iamed2 commented on ec427e8 Mar 23, 2022

Choose a reason for hiding this comment

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

I was just about to work on something that uses >1d PooledArrays with specified reftype :) Any chance for a quick tag?

@bkamins
Copy link
Member Author

Choose a reason for hiding this comment

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

I am waiting for a review of #79 and then I think we can tag a patch release. (so hopefully this week)

@nalimilan - what do you think?

@bkamins
Copy link
Member Author

Choose a reason for hiding this comment

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

@iamed2 - for the time being you can just check out #main branch in your project as it should be 100% safe (noting in main is unsafe).

@nalimilan
Copy link
Member

Choose a reason for hiding this comment

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

Maybe tag a new release now? Anyway README.md is mainly read on GitHub and no release is needed for that. I'll have a look at #79 nevertheless but maybe not today. :-)

Please sign in to comment.