Skip to content

update extension yamls#51

Merged
alessandrofelder merged 7 commits intomasterfrom
update-extension-yamls
Jun 1, 2020
Merged

update extension yamls#51
alessandrofelder merged 7 commits intomasterfrom
update-extension-yamls

Conversation

@alessandrofelder
Copy link
Collaborator

@alessandrofelder alessandrofelder commented May 29, 2020

The files silverlab.*.yaml are generated by generate_extended_schema.py
but weren't up-to-date on the repo.

Only changes are small typo fixes inside strings. This affects the signatures, however!

Also, fixes problems with newer versions of tifffile, which don't include the indirect dependency imagecodecs by default anymore.

these files are generated by generate_extended_schema.py
but weren't up-to-date on the repo.

Only changes are small typo fixes inside strings.
@alessandrofelder alessandrofelder requested a review from ageorgou May 29, 2020 09:44
@alessandrofelder alessandrofelder marked this pull request as draft June 1, 2020 09:13
shape:
- 2
doc: ' the 2d imaging frame size in voxels'
doc: the 2d imaging frame size in voxels
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to these changes, but is this really voxels if we're imaging in 2d?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, it's debatable, I think... I could be wrong, but you could argue that since we are looking at a 3D image stack overall the data points we are looking at are voxels, and each imaging frame has, therefore, frame_size[0] voxels in one direction, and frame_size[1] voxels in the other. Guess it depends whether you want to emphasize the fact that the data points are 3d or that the frame is 2d more strongly?

Copy link
Contributor

@ageorgou ageorgou left a comment

Choose a reason for hiding this comment

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

Looks reasonable, just need to update the signatures to match!

these needed updating due to both:
 * the silverlab extension having changed (typo fix in
   `generate_extended_schema.py` was not propagated to extension yaml files)
 * the previous changes to metadata like switching to `ruamel.yaml` and writing pynwb defaults to file instead of parameter explanations
@alessandrofelder alessandrofelder marked this pull request as ready for review June 1, 2020 14:33
@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2020

Codecov Report

Merging #51 into master will decrease coverage by 0.06%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage   53.76%   53.69%   -0.07%     
==========================================
  Files          12       12              
  Lines        1581     1583       +2     
  Branches      252      253       +1     
==========================================
  Hits          850      850              
- Misses        696      697       +1     
- Partials       35       36       +1     
Impacted Files Coverage Δ
src/silverlabnwb/generate_extended_schema.py 0.00% <ø> (ø)
tests/test_metadata_import.py 94.73% <0.00%> (-5.27%) ⬇️

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 9261d04...8027996. Read the comment docs.

@ageorgou
Copy link
Contributor

ageorgou commented Jun 1, 2020

Not sure why the Travis tests aren't linked properly, but they're passing.

@alessandrofelder alessandrofelder merged commit cfd91b0 into master Jun 1, 2020
@ageorgou ageorgou deleted the update-extension-yamls branch July 23, 2020 15:29
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.

3 participants