CottageLabs / LanternPM

Lantern meta repository for product management
1 stars 0 forks source link

Where values cannot be meaninfully set, omit them from the API result #111

Open richard-jones opened 8 years ago

richard-jones commented 8 years ago

For example, in romeo results:

embargo: {
    preprint: false,
postprint: false,
pdf: false
},
archiving: {
preprint: false,
postprint: false,
pdf: false
}

Better to omit the "false", as otherwise you need to resort to type-checking on the client side to correctly interpret the values.

Otherwise, if the keys must be present, better to set them to null if the value is not known.

markmacgillivray commented 8 years ago

I don't think I understand how they are not already meaningfully set. False has meaning. I can't omit the false, or do you mean omit the keys as well if the value is false? Then you would still have to check for key existence, which surely is the same as checking for value === false. Similarly, how does that lack clarity compared to checking value === null? These could be changed, but I need more understanding first - @richard-jones please provide more info, ta.

richard-jones commented 8 years ago

Well, I don't have any documentation for this bit of the API, so I don't know whats meaningful but:

self-archiving policy from sherpa consists of the string "can", "cannot" or "restricted". So does false mean "cannot"? Or does it mean "we didn't get an answer", or "we didn't check".

Likewise, embargo is numeric, so does false mean that there is no embargo? or does it mean we don't know?

markmacgillivray commented 8 years ago

This bit has never been used by anyone else. So far we have done what wellcome required, and at that point they only used it by taking the csv dumps rather than the raw API data.

false is the default state, and represents that we don't know if the answer is any of the ones we otherwise say it would be.

So this seems to be a matter for documentation / use case choice rather than changing the data in a particular way to meet any known requirement, although we could change the data too.

I don't think having null vs false would be much better - we'd still have to document that null means we don't know if it is one of the values it should be, rather than itself being a value.

My preference would be to not even show the keys if we do not have useful information to share, then people can check for key existence, and if present then it means we have useful information to act on.

Any other preferences?

On Mon, Aug 22, 2016 at 11:06 AM, Richard Jones notifications@github.com wrote:

Well, I don't have any documentation for this bit of the API, so I don't know whats meaningful but:

self-archiving policy from sherpa consists of the string "can", "cannot" or "restricted". So does false mean "cannot"? Or does it mean "we didn't get an answer", or "we didn't check".

Likewise, embargo is numeric, so does false mean that there is no embargo? or does it mean we don't know?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CottageLabs/LanternPM/issues/111#issuecomment-241368747, or mute the thread https://github.com/notifications/unsubscribe-auth/AAuXCDL0ddozrT5JtGBn6muvQg_FHxpkks5qiXTAgaJpZM4JmoYC .

emanuil-tolev commented 8 years ago

My preference would be to not even show the keys if we do not have useful information to share, then people can check for key existence, and if present then it means we have useful information to act on.

:+1: to that

richard-jones commented 8 years ago

Yes, me too, that makes sense.