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.

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

Implementing repeat for PooledArray #67

Merged
merged 9 commits into from
May 17, 2021
Merged

Implementing repeat for PooledArray #67

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f913260
Implementing repeat for PooledArray
sl-solution May 8, 2021
3391ecd
update the repeat() function
sl-solution May 8, 2021
5ad8abf
add tests for repeat
sl-solution May 8, 2021
901c26b
Update PooledArrays.jl
sl-solution May 8, 2021
8cc60f6
fix the problem with Julia 1.0.5
sl-solution May 9, 2021
f8818d9
Update test/runtests.jl
sl-solution May 16, 2021
6f6610e
Update src/PooledArrays.jl
sl-solution May 16, 2021
d8acef4
Update runtests.jl
sl-solution May 16, 2021
5600e9b
Fix tests
nalimilan May 17, 2021
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
3 changes: 3 additions & 0 deletions src/PooledArrays.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,4 +601,7 @@ _perm(o::F, z::V) where {F, V} = Base.Order.Perm{F, V}(o, z)

Base.Order.Perm(o::Base.Order.ForwardOrdering, y::PooledArray) = _perm(o, fast_sortable(y))

Base.repeat(x::PooledArray, counts...) = PooledArray(RefArray(repeat(x.refs, counts...)), copy(x.invpool))
Copy link
Member

Choose a reason for hiding this comment

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

copying of pool and invpool is not needed and should be avoided. Please check the implementation of other operations to see how copy-on-write is implemented. Thank you!

Copy link
Member

Choose a reason for hiding this comment

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

also could you please add some tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for comments. Updated the function and add some tests.

Copy link
Member

Choose a reason for hiding this comment

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

Ah - now I see. Such ambiguities are super annoying (and that is why having a detailed test coverage is crucial 😄). Thank you!

Base.repeat(x::PooledArray; inner = nothing, outer = nothing) = PooledArray(RefArray(repeat(x.refs; inner = inner, outer = outer)), copy(x.invpool))

end