Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENH] Microscopy: Add IntendedFor metadata field to photo files #1000

Merged

Conversation

mariehbourget
Copy link
Collaborator

@mariehbourget mariehbourget commented Feb 4, 2022

closes #992

Dear BIDS community,

This PR address the issue in:

... where there was no mechanism in microscopy to associated a photo of a sample to a specific run or modality (for example: a photo could be generated at each run). Our best option seems to be a JSON file associated to the photo with the IntendedFor metadata field, which I added in this PR.

Link to the rendered doc: https://bids-specification--1000.org.readthedocs.build/en/1000/04-modality-specific-files/10-microscopy.html#photos-of-the-samples-_photoextension

Please let me know your thoughts and suggestions, thank you!

Note: If we go with this, we will need to update the bids-validator with the addition of the JSON file for photo and its associated rules.

@codecov
Copy link

codecov bot commented Feb 4, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.11%. Comparing base (4e9a872) to head (a402787).
Report is 1250 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1000      +/-   ##
==========================================
+ Coverage   70.16%   71.11%   +0.94%     
==========================================
  Files           9        9              
  Lines         952      952              
==========================================
+ Hits          668      677       +9     
+ Misses        284      275       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sappelhoff sappelhoff added this to the 1.7.0 milestone Feb 4, 2022
@satra
Copy link
Collaborator

satra commented Feb 4, 2022

while we are making changes to the json document, would there be some benefit to add the possibility of some field that describes details of acquisition.

examples:

  • one can take a photo before and after clearing
  • block-face image and then slice laid out image

at the simplest level an optional description field can be added.

cc: @LeeKamentsky @gmazzamuto

@LeeKamentsky
Copy link

This is kind of long-range, but it might be nice to place the photo in time with relation to the tissue protocol - somehow linking it to between two stages or after a stage. @satra is right about taking the photo before or after clearing - a huge difference in appearance and the intention of the photographer in picking the timing of the photo. I can imagine taking a picture of one of our slabs on the microscope stage would help us understand the orientation of the resulting image and other aspects.

@mariehbourget
Copy link
Collaborator Author

Great idea, thanks! I added a "PhotoDescription" field with example.

@mariehbourget
Copy link
Collaborator Author

I opened WIP PR for bids-examples (bids-standard/bids-examples#307) and bids-validator (bids-standard/bids-validator#1415) associated with the changes.

@sappelhoff sappelhoff removed this from the 1.7.0 milestone Feb 9, 2022
@gmazzamuto
Copy link

That's seems a good idea! indeed we usually take photos of the samples before and after clearing. These were also uploaded to DANDI

@mariehbourget
Copy link
Collaborator Author

Hi everyone, it's been a while!
We are currently finalizing the validator PR bids-standard/bids-validator#1415 associated with these changes.

So this PR is ready for review again.
As discussed, it adds:

  • A JSON sidecar file to _photo files.
  • 2 fields in this JSON: PhotoDescription and IntendedFor.

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other modalities where "_photo" is used as a suffix file apparently don't (yet?) allow .json sidecars, but I see no reason why it shouldn't be the case.

LGTM

@sappelhoff
Copy link
Member

@TheChymera I wanted to request you as a reviewer and then noticed that you are not yet part of the org, so I've sent you some invitations for that. Please check your inbox :-)

Copy link
Collaborator

@Remi-Gau Remi-Gau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sappelhoff sappelhoff changed the title [ENH] - Microscopy: Add IntendedFor JSON field to photo files [ENH] Microscopy: Add IntendedFor JSON field to photo files Jul 13, 2022
@sappelhoff sappelhoff added this to the 1.8.0 milestone Jul 13, 2022
@sappelhoff sappelhoff merged commit 130aa76 into bids-standard:master Jul 19, 2022
@effigies effigies changed the title [ENH] Microscopy: Add IntendedFor JSON field to photo files [ENH] Microscopy: Add IntendedFor metadata field to photo files Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microscopy files of suffix _photo do not support a _run- entity.
6 participants