[RFC] Before trying to lower() an AbstractArray, replace undef values with a defined value of the same type - fixes #3

[DilumAluthge]

This is a really hacky way to solve #3. The basic idea is this: before trying to lower() an AbstractArray, we go through the array and replace all the #undef values with a value of the appropriate type. (Specifically, we replace the #undef values with the first defined value in the array.)

For example, if we have an array that looks like this:

5-element Array{String,1}:
 #undef
    "b"
 #undef
    "d"
    "e"

Then, before we try to lower() it, we change the #undef values so the array looks like this:

5-element Array{String,1}:
    "b"
    "b"
    "b"
    "d"
    "e"

It seems like those values are ignored, so it doesn't matter what we set them to, as long as we set them to values of the appropriate type.

[DilumAluthge]

Requesting review @MikeInnes

[MikeInnes]

This doesn't seem ideal, because presumably those values will then re-appear when you try to load the file?

I think it'd be better to insert some kind of sentinel when writing the array, and use that when reading back in.

[DilumAluthge]

I believe that those values are ignored when reading back from file, but honestly I’m not sure. What kind of sentinel value would we use? It would have to be of the same type, right? Alternatively we could create a new Singleton type and use that, but then we’d have to change the type of the Array to either Array{Any} or Array{Union{DummyType, T}} where T is the original eltype

On Thu, May 31, 2018 at 11:06 Mike J Innes ***@***.***> wrote: This doesn't seem ideal, because presumably those values will then re-appear when you try to load the file? I think it'd be better to insert some kind of sentinel when writing the array, and use that when reading back in. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFXArSQtkOvE-xyGnBEdb47MLZldXYciks5t4Ab3gaJpZM4UOwCa> .

-- *Dilum Aluthge* *[email protected] <[email protected]>* *https://www.aluthge.com <https://www.aluthge.com>*

[MikeInnes]

I mean at a lower level, i.e. when we write an array out as bytes in the file. e.g. we could say that a zero byte means undefined, and then the reader won't fill the array where it sees a zero. (Just an example, I'm not sure that would work).

[DilumAluthge]

Ahh I see. Seems like it would work, as long as a zero byte doesn’t already have another meaning in the BSON spec.

On Thu, May 31, 2018 at 11:58 Mike J Innes ***@***.***> wrote: I mean at a lower level, i.e. when we write an array out as bytes in the file. e.g. we could say that a zero byte means undefined, and then the reader won't fill the array where it sees a zero. (Just an example, I'm not sure that would work). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFXArd77SFcc1nXcVXdtMm2V2v60HUd1ks5t4BMhgaJpZM4UOwCa> .

-- *Dilum Aluthge* *[email protected] <[email protected]>* *https://www.aluthge.com <https://www.aluthge.com>*

[DilumAluthge]

So we actually already have `undefined <: BSONType`. We could probably just use that?

On Thu, May 31, 2018 at 12:05 Dilum Aluthge ***@***.***> wrote: Ahh I see. Seems like it would work, as long as a zero byte doesn’t already have another meaning in the BSON spec. On Thu, May 31, 2018 at 11:58 Mike J Innes ***@***.***> wrote: > I mean at a lower level, i.e. when we write an array out as bytes in the > file. e.g. we could say that a zero byte means undefined, and then the > reader won't fill the array where it sees a zero. (Just an example, I'm not > sure that would work). > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#6 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AFXArd77SFcc1nXcVXdtMm2V2v60HUd1ks5t4BMhgaJpZM4UOwCa> > . > -- *Dilum Aluthge* ***@***.*** ***@***.***>* *https://www.aluthge.com <https://www.aluthge.com>*

-- *Dilum Aluthge* *[email protected] <[email protected]>* *https://www.aluthge.com <https://www.aluthge.com>*

[DilumAluthge]

Actually, it turns out that "undefined" has been deprecated in the BSON spec.

[DilumAluthge]

More importantly: I don't think that would work. The reference to undefined happens when we call lower(x::Array), which is well before we start writing bytes to file. So we have to fix the undefineds before or during the call to lower(x::Array).

[DilumAluthge]

I'm closing this - I think I've found a way to avoid generating these arrays with #undef values in the first place. I'll make a PR soon.

[MikeInnes]

Right, so lower could insert undefined (or some other sentinel value, it doesn't have to be that specifically).

[MikeInnes]

Actually, that probably isn't quite right if you want to preserve types. But still, lower can just let undefined's through rather than throwing, and the final write can insert the sentinel.

[DilumAluthge]

So I tried to get this to work, but the problem I ran into was: before writing an array, we splat it a few times, and if you try to splat an array with undefineds, you get the “undefined resource” error. So we’d have to do something about the undefineds before we splat the error. I couldn’t get this to work, so I decided to work on the issue causing us to have undefineds in arrays in the first place, I.e. the problem with writing dicts.

On Mon, Jun 25, 2018 at 06:23 Mike J Innes ***@***.***> wrote: Actually, that probably isn't quite right if you want to preserve types. But still, lower can just let undefined's through rather than throwing, and the final write can insert the sentinel. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#6 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFXArW-cAJdJ7Zj0X-FOBHF8IAhZFLojks5uALoPgaJpZM4UOwCa> .

-- *Dilum Aluthge* *[email protected] <[email protected]>* *https://www.aluthge.com <https://www.aluthge.com>*