add json validation to tests#310
Conversation
for more information, see https://pre-commit.ci
…nto validate_read_probe_dicts
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #310 +/- ##
==========================================
+ Coverage 89.42% 89.46% +0.04%
==========================================
Files 10 11 +1
Lines 1891 1899 +8
==========================================
+ Hits 1691 1699 +8
Misses 200 200 ☔ View full report in Codecov by Sentry. |
|
Here I just added a line to each of the tests. This works, but there might be a more elegant solution, for example to use parameterized tests. Feel free to refactor for style |
for more information, see https://pre-commit.ci
…nto validate_read_probe_dicts
for more information, see https://pre-commit.ci
| "type": "object", | ||
| "properties": { | ||
| "name": { "type": "string" }, | ||
| "model_name": { "type": "string" }, |
There was a problem hiding this comment.
I wasn't sure about this name change. Both "name" and "model_name" are present as metadata, and neither required by the probe object. Should we make it required here?
There was a problem hiding this comment.
Do we want the json_schema to map perfectly onto the Probe object, or might it make sense of have them hav slightly different requirements?
There was a problem hiding this comment.
Thanks @bendichter
I think it makes sense to have model_name and not name as required. name is a custom name that can be defined for a probe (e.g., ProbeA-B-C for Open Ephys system), but definitely not a requirement.
|
thanks a lot Ben. I will have a look. |
alejoe91
left a comment
There was a problem hiding this comment.
Added also tests for Open Ephys and SpikeGadgets, so testing is complete :)
I added some json schema validation to the tests and found a few minor discrepancies in the json schema