Add functionality to use arbitrary singular values in ConcatDiskArrays#256
Conversation
| function ConcatDiskArray(arrays::AbstractArray{Union{<:AbstractArray,Missing}}; fill=missing) | ||
| et = Base.nonmissingtype(eltype(arrays)) | ||
| T = Union{Missing,eltype(et)} | ||
| T = promotetype(typeof(fill), eltype(et)) |
There was a problem hiding this comment.
I thought the passed in array would still contain either arrays or missing values, but we would just return fill for those sections, so fill would need to be a struct field.
So what we have here is kinda half way. Probably we should either drop the fill keyword (and just rely on the array values) or keep it as a struct field and use missing in the array.
There was a problem hiding this comment.
I prefer to drop the fill keyword. I actual thought I already did so.
With my approach we can have different fill values in different parts of the concatenated array.
There was a problem hiding this comment.
But is that actually useful? I can't imagine a real use case for multiple fill values. And putting missing in the array seems clearer for representing missing arrays.
The missing value used in the array of arrays doesn't have to be coupled to the fill value.
There was a problem hiding this comment.
I agree that it might be a bit dangerous to allow arbitrary values to represent missings. OTOH one never knows when this might become useful for some reason. I am ok with both ways and would currently slightly prefer @felixcremer s solution because there is already an implementation. If we want to be a bit more on the safe side, we could introduce a small wrapper type like
struct MissingTile{F}
fillvalue::F
endthat explicitly marks missing entries to be concatenated and how they should be filled. The main reason why the current solution might be problematic is for DiskArrays with arrays as eltype, but maybe this is overthinking things a bit too much.
Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
| function ConcatDiskArray(arrays::AbstractArray{Union{<:AbstractArray,Missing}}; fill=missing) | ||
| et = Base.nonmissingtype(eltype(arrays)) | ||
| T = Union{Missing,eltype(et)} | ||
| T = promotetype(typeof(fill), eltype(et)) |
There was a problem hiding this comment.
| T = promotetype(typeof(fill), eltype(et)) | |
| T = Union{Missing,eltype(et)} |
There was a problem hiding this comment.
Don't you have to read all the fill values in the array and promote them? Missing is not necessarily in the type?
Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
|
Why do the tests not run on this pr? |
|
I took this for a spin and I am not sure, whether it is this change or #256 or ConcatDiskArray even before but saving a ConcatDiskArray where I have some missings takes a very long time. Saving the european dataset took the whole weekend. |
This introduces a MissingTile type to wrap the values for ConcatDiskArray. This allows to use vectors as elements in the fillvalue.
|
I will merge this for now, as it is only a PR against my other PR. We can continue discussing in #254 |
3 participants
This adds the possibility to use any value instead of only missing in ConcatDiskArrays.
We can even mix the singular values.