loose eltype when fillvalue is nothing or missing#24
loose eltype when fillvalue is nothing or missing#24johnnychen94 merged 2 commits intoJuliaArrays:masterfrom johnnychen94:jc/nothing
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24 +/- ##
==========================================
+ Coverage 71.42% 72.09% +0.66%
==========================================
Files 1 1
Lines 42 43 +1
==========================================
+ Hits 30 31 +1
Misses 12 12
Continue to review full report at Codecov.
|
|
I don't think you have to convert anything; you can just use T = eltype(data)
T = fillvalue isa Union{Missing,Nothing} ? Union{typeof(fillvalue),T} : Tin the constructor and use that for the first type parameter of |
|
🤔 looks like you're right; the padded value doesn't come from Will change accordingly when I find some time. |
johnnychen94
changed the title
| function PaddedView{T,N,I,A}(fillvalue, | ||
| data, | ||
| indices::NTuple{N,AbstractUnitRange}) where {T,N,I,A} | ||
| ndims(data) == N || throw(DimensionMismatch("data and indices should have the same dimension, instead they're $(ndims(data)) and $N.")) |
There was a problem hiding this comment.
Now we can't ensure that eltype(data) == T
|
That was fast, thanks so much for working on this! Much appreciated. |
|
I think the revision commit does exactly what @timholy indicated, so I'm merging it now |
|
Yep, perfect to the letter. Really nice work! And thanks for doing it. ❤️ |
4 participants
Closes #19
Because for Nothing and Missing, T isn't the eltype anymore, we have
to add several methods(eltype, similar) to support it.
This might be good to go, but I'll need to check it more carefully. I was hoping to provide a patch before anyone sees the updates in #19.