Add DatasetLinks to JSON schema #1404
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1404 +/- ##
=======================================
Coverage 84.10% 84.10%
=======================================
Files 90 90
Lines 3631 3631
Branches 1098 1098
=======================================
Hits 3054 3054
Misses 483 483
Partials 94 94 Continue to review full report at Codecov.
|
|
The error message for the empty string is hard to interpret. Can we just make the format of the key to be alphanumeric of length >=1? |
|
It may be possible using |
| "minLength": 1, | ||
| "pattern": "^[a-zA-Z0-9]*$" |
There was a problem hiding this comment.
What about:
| "minLength": 1, | |
| "pattern": "^[a-zA-Z0-9]*$" | |
| "pattern": "^[a-zA-Z0-9]+$" |
Or would that make the error less clear?
There was a problem hiding this comment.
yes, that was my train of thought: better to be explicit here. Everyone can guess what minLength means, but few people know the difference between * and + in a regex
There was a problem hiding this comment.
don't know about the error message though (on re-reading, that seems to be your concern, whereas mine was with future devs)
There was a problem hiding this comment.
it would change .DatasetLinks should NOT be shorter than 1 characters to '.DatasetLinks should match pattern "^[a-zA-Z0-9]+$"'
There was a problem hiding this comment.
My concern is that we get both:
'.DatasetLinks should NOT be shorter than 1 characters'
".DatasetLinks property name '' is invalid"
The second one seems more helpful.
There was a problem hiding this comment.
My concern is that we get both
I see what you mean, but that's also the case after your suggested fix unfortunately. It'd just be
'.DatasetLinks should match pattern "^[a-zA-Z0-9]+$"'
".DatasetLinks property name '' is invalid"
instead.
I think both of these error messages are helpful though (much more helpful than before in any case, so thanks for your feedback)
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
bids-validator/validators/json/schemas/dataset_description.json
Outdated
Show resolved
Hide resolved
bids-validator/tests/json.spec.js
Outdated
| DatasetLinks: { | ||
| mylink1: 'https://www.google.com', | ||
| mylink2: 1, | ||
| mylink3: 'nope', |
There was a problem hiding this comment.
I guess this is now valid. Can either drop it or make it invalid.
| mylink3: 'nope', | |
| mylink3: ':/nope', |
There was a problem hiding this comment.
I'll drop it and change it to a test that fails for keys like I am a key with whitespace and not alphanumeric "§%
* first try * add functional DatasetLinks schema: any str bigger than len 0 --> uri * add tests * fix lint * use propertyNames feature, instead of not feature * migrated eeg_matchingpennies from GitHub to GIN * fix formatting E501 * Update bids-validator/tests/json.spec.js Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Update bids-validator/validators/json/schemas/dataset_description.json * fix tests Co-authored-by: Chris Markiewicz <effigies@gmail.com>
addresses point one of #1393: Adding the
DatasetLinksmetadata field to thedataset_description.jsonJSON schema.xref bids-standard/bids-specification#918