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

Add images tutorial #1470

Merged
merged 36 commits into from
Jul 18, 2022
Merged

Add images tutorial #1470

merged 36 commits into from
Jul 18, 2022

Conversation

weiglszonja
Copy link
Collaborator

@weiglszonja weiglszonja commented May 4, 2022

Motivation

Create a new tutorial that demonstrates how to use the image module for adding images to an NWB File.

The images tutorial covers the following data types:

  • pynwb.image.GrayscaleImage, pynwb.image.RGBImage, pynwb.image.RGBAImage
  • pynwb.image.OpticalSeries
  • pynwb.image.ImageSeries,
  • pynwb.image.ImageMaskSeries ,
  • pynwb.image.IndexSeries (which was changed recently)

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Have you checked our Contributing document?
  • Have you ensured the PR clearly describes the problem and the solution?
  • Is your contribution compliant with our coding style? This can be checked running flake8 from the source directory.
  • Have you checked to ensure that there aren't other open Pull Requests for the same change?
  • Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

@weiglszonja weiglszonja marked this pull request as draft May 4, 2022 13:09
@weiglszonja weiglszonja self-assigned this May 4, 2022
@bendichter bendichter marked this pull request as ready for review July 13, 2022 12:06
@bendichter
Copy link
Contributor

@weiglszonja let's move forward with this. I think the content we already have here is good. It doesn't cover everything yet but let's get this out before the user days to increase our tutorial coverage

@codecov
Copy link

codecov bot commented Jul 13, 2022

Codecov Report

Merging #1470 (b44598e) into dev (1315a0d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev    #1470   +/-   ##
=======================================
  Coverage   90.52%   90.52%           
=======================================
  Files          25       25           
  Lines        2460     2460           
  Branches      456      456           
=======================================
  Hits         2227     2227           
  Misses        148      148           
  Partials       85       85           
Flag Coverage Δ
integration 69.87% <ø> (ø)
unit 83.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out 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 1315a0d...b44598e. Read the comment docs.

@bendichter
Copy link
Contributor

This content would need to be removed from NWBFile basics

unit="n.a.",
external_file=external_file,
format="external",
starting_frame=[0],
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use multiple movie files, I believe you would need a starting_frame value for each file.

Copy link
Contributor

@oruebel oruebel Jul 15, 2022

Choose a reason for hiding this comment

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

I'm not sure how many frames are in each movie file, but assuming "file_1.tiff" and "file_2.tiff" contain 2 frames and "file_2.tiff" contains 3 frames, I believe the correct value for starting_frame should bestarting_frame=[0, 2, 6],

According to the schema of ImageSeries:

        Each external image may contain one or more consecutive frames of the full
        ImageSeries. This attribute serves as an index to indicate which frames each file
        contains, to faciliate random access. The 'starting_frame' attribute, hence,
        contains a list of frame numbers within the full ImageSeries of the first frame
        of each file listed in the parent 'external_file' dataset. Zero-based indexing is
        used (hence, the first element will always be zero). For example, if the
        'external_file' dataset has three paths to files and the first file has 5 frames,
        the second file has 10 frames, and the third file has 20 frames, then this
        attribute will have values [0, 5, 15]. If there is a single external file that
        holds all of the frames of the ImageSeries (and so there is a single element in
        the 'external_file' dataset), then this attribute should have value [0].

Copy link
Collaborator

Choose a reason for hiding this comment

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

Related to #1318 - if we get an answer on that issue I can close it

Copy link
Contributor

Choose a reason for hiding this comment

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

@CodyCBakerPhD thanks for pointing us to this issue. I just added a response based on my understanding of the schema. I hope this helps clarify things a bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@oruebel thank you for this example! I'll extend the external file section with this description (or a shorter version of it) to make this clear.

oruebel
oruebel previously approved these changes Jul 18, 2022
Copy link
Contributor

@oruebel oruebel left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for creating this tutorial and for your patients in iterating on edits.

@weiglszonja
Copy link
Collaborator Author

@oruebel I learnt a lot from the suggestions, thank you for them! Now only the thumbnail image is missing, do you have an idea how it should look like?

@oruebel
Copy link
Contributor

oruebel commented Jul 18, 2022

Now only the thumbnail image is missing, do you have an idea how it should look like?

That's great! The PowerPoint file with the source of all the other thumbnails is here
https://github.com/NeurodataWithoutBorders/pynwb/blob/dev/docs/source/figures/gallery_thumbnails.pptx . Just copy one of the existing thumbnails to a new slide, update the title, and add a small image. For the picture I would maybe either: a) add an actual photo from an experiment or b) just take the mouse from the "Behavior Data" thumbnail and add a clip-art of a camera taking a photo of the mouse. But just have fun and be creative with the image.

@weiglszonja
Copy link
Collaborator Author

@oruebel how about this?
gallery_thumbnails_image_data

@oruebel
Copy link
Contributor

oruebel commented Jul 18, 2022

how about this?

Looks great!

oruebel
oruebel previously approved these changes Jul 18, 2022
Copy link
Contributor

@oruebel oruebel left a comment

Choose a reason for hiding this comment

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

This looks all good to me! Thanks for the hard work!

@weiglszonja
Copy link
Collaborator Author

@oruebel @bendichter thank you for the suggestions!

@bendichter bendichter merged commit 47b92d3 into dev Jul 18, 2022
@oruebel oruebel deleted the docs/add_images_tutorial branch July 18, 2022 22:23
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.

4 participants