[ENH] Microscopy: Add validation of JSON for photo files

[mariehbourget]

This PR adds validation of the new JSON file associated with microscopy photo files in:

• [ENH] Microscopy: Add IntendedFor metadata field to photo files bids-specification#1000.

Still WIP as the spec PR is not approved yet.

The new JSON file includes the IntendedFor field.
I used the same error code (37) and validation steps as in nii.js here.

New photos and associated JSON files are added in microscopy examples in:

• [ENH] Microscopy: Add photos and associated JSON files bids-examples#307.

[rwblair]

These changes look good. I'm looking at a way to combine the logic from different parts of the validator. I'll update this PR accordingly depending on what I find.

[mariehbourget]

Hi @rwblair, is there any news on this PR?
Thanks!

[rwblair]

@mariehbourget I made a PR against this branch:
neuropoly#8

Adds some tests and rebases on master. I also tested against the bids-examples branch and it worked as expected.Once tests are passing for this I'll go ahead and merge it.

[mariehbourget]

Thanks @rwblair!
Just to make sure I understand correctly, you need me to merge neuropoly#8 first?

As for merging to the bids-validator master, we may want to wait for bids-standard/bids-specification#1000 to be approved, I'll ask for a final review.

[rwblair]

Just to make sure I understand correctly, you need me to merge neuropoly#8 first?

That be great, depending on how the CI does I may make more PRs.

Waiting for the specification merge to happen first isn't a problem.

[mariehbourget]

Hi @rwblair!
I merged the PR (neuropoly 8) and the specification PR bids-standard/bids-specification#1000 was approved last week, so I think we are OK to proceed.

There are some errors with the last merge that I don't fully understand, let me know if there is anything I can do to help.
Thanks!

[sappelhoff]

@mariehbourget is the box "allow commits from maintainers" not checked? I just wanted to push a fix but couldn't do it.

Anyhow, fixes are in neuropoly#9, which you can simply merge :-) --> but in the future it would be nice to tick the "allow commits from maintainers" box

[codecov]

Codecov Report

Merging #1415 (3e83193) into master (4feb91b) will increase coverage by 0.84%.
The diff coverage is 73.91%.

@@            Coverage Diff             @@
##           master    #1415      +/-   ##
==========================================
+ Coverage   84.10%   84.95%   +0.84%     
==========================================
  Files          90       90              
  Lines        3631     3683      +52     
  Branches     1098     1121      +23     
==========================================
+ Hits         3054     3129      +75     
+ Misses        483      467      -16     
+ Partials       94       87       -7     

Impacted Files	Coverage Δ
bids-validator/src/options.js	100.00% <ø> (ø)
bids-validator/utils/options.js	70.83% <ø> (ø)
...ds-validator/validators/microscopy/checkSamples.js	100.00% <ø> (ø)
...validator/validators/tsv/validateContRecordings.js	100.00% <ø> (ø)
...ids-validator/validators/tsv/validateTsvColumns.js	52.56% <30.76%> (-1.08%)	⬇️
bids-validator/utils/issues/index.js	79.54% <50.00%> (-0.69%)	⬇️
bids-validator/validators/json/json.js	98.29% <50.00%> (-0.83%)	⬇️
bids-validator/utils/files/readDir.js	78.06% <78.94%> (-0.15%)	⬇️
...lidator/validators/microscopy/checkJSONAndField.js	92.98% <88.23%> (-4.17%)	⬇️
bids-validator/validators/microscopy/ometiff.js	72.97% <90.00%> (-0.64%)	⬇️
... and 23 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4feb91b...3e83193. Read the comment docs.

[mariehbourget]

Anyhow, fixes are in neuropoly#9, which you can simply merge :-) --> but in the future it would be nice to tick the "allow commits from maintainers" box

Hi @sappelhoff, thanks for the PR neuropoly#9, it is now merged.

I would gladly grant you access of course but I could not find the "allow commits from maintainers" box anywhere, sorry about that. Is it supposed to be on the main page of the PR (this page)?

[sappelhoff]

see the lower right of this screenshot:

or simply: Open a pull request of yours and search for "Allow edits and access to secrets by maintainers" or subsets of this string (just via ctrl+F search in your browser)

[mariehbourget]

Hum, that's what I thought from the github help pages but I have nothing under the "participants":

I'll keep looking and check if there is something in our fork settings.

[sappelhoff]

Yes, perhaps it's because you submitted the PR from a "third party" organization account 🤔 ... either the situation there is completely different OR you have insufficient rights there 🤷‍♂️ anyhow the solution is probably to next time do a PR from you personal fork :-)