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.

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

#undef in SentinelArray #26

Open
bkamins opened this issue Aug 4, 2020 · 4 comments
Open

#undef in SentinelArray #26

bkamins opened this issue Aug 4, 2020 · 4 comments

Comments

@bkamins
Copy link
Member

bkamins commented Aug 4, 2020

This is a follow up to JuliaData/Tables.jl#186 to keep track of the issue.

@quinnj - does it make sense to define a custom isassigned for SentinelArray? I think it is not a problem and should simplify things to add it, but you know the design much better.

@quinnj
Copy link
Member

quinnj commented Aug 16, 2020

Do you have a specific idea for what isassigned would mean here? Like, are you thinking it would check for #undef, even if it's used as the sentinel, like for SentinelArray(["hey", "there"])? The isassigned fallback works already, but we could define Base.isassigned(A::SentinelArray, i) = isassigned(parent, i) to "pass-through" to the more efficient Array implementation under the hood.

@bkamins
Copy link
Member Author

bkamins commented Aug 16, 2020

We probably misunderstood the SentinelArray design there. So in practice if parent has a possibility of #undef (i.e. it is not a bits type) it will always get turned to missing and be used as a sentinel - right?

If so (i.e. it is not possible to have #undef in SentinelArray) isassigned could just do what Base.checkbounds returns I think and then isassigned would be fast.

@quinnj
Copy link
Member

quinnj commented Aug 17, 2020

You can still have #undef in SentinelArray, but you would have to use your own sentinel manually, i.e. by default, reference types will use #undef as their sentinel. So, for example, you can still do SentinelArray(Vector{String}(undef, 3), "custom_sentinel") and get #undef elements.

@bkamins
Copy link
Member Author

bkamins commented Aug 17, 2020

I guess we could check:

  1. if isbitstype || sentinel is #undef - it is enough to check range else fall back to the default implementation (it should be rare in practice)

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

No branches or pull requests

2 participants