type guards for SPAnalysisDataModel #93
Conversation
| export const isSamplingOpts = (x: any): x is SamplingOpts => { | ||
| if (!x) return false | ||
| if (typeof x !== 'object') return false | ||
| if (typeof x.num_chains !== 'number') return false | ||
| if (typeof x.num_warmup !== 'number') return false | ||
| if (typeof x.num_samples !== 'number') return false | ||
| if (typeof x.init_radius !== 'number') return false | ||
| if (x.seed !== undefined && typeof x.seed !== 'number') return false | ||
| return true | ||
| } |
There was a problem hiding this comment.
Should we also verify that e.g. num_chains is an integer?
Not sure what the best way to check this is in JS -- Math.round(x) === x?
There was a problem hiding this comment.
We could check that elsewhere, but I don't think this belongs in the type guard. I believe a type guard should have the sole purpose of guaranteeing that the thing is of the right javascript type.
| if (isSamplingOpts(parsed)) { | ||
| data.samplingOpts = parsed | ||
| } | ||
| else { | ||
| console.error('Invalid sampling_opts in fetchRemoteAnalysis', parsed) | ||
| } |
There was a problem hiding this comment.
If we're coming this far, it'd probably be good to include a validity check beyond just a type-guard--e.g. natural numbers where appropriate. (I'm okay with the discussion below that suggests it shouldn't be in the type guard proper, but I think if we're going to do the deserialization validity check it's worth it to go the whole way in another fn)
There was a problem hiding this comment.
I'm imagining most likely a formal parsing function, not in this file but closer to the type definition, that would replace the JSON.parse() line above and return either a valid SamplingOpts or undefined?
Fixes #89
Added some type guards which are used to check the input of local storage and also in
loadFromProjectFilesThis was somewhat difficult to test because if you manually put an error in the local storage, that gets overwritten upon page reload. So I managed to achieve this test by putting a temporary error in the write-to-local-storage code and verified that the page did not crash and I got the warning message upon reload.