Skip to content
This repository has been archived by the owner on Mar 15, 2023. It is now read-only.

Correct order of writing in add_two_photon_series #547

Closed
wants to merge 4 commits into from

Conversation

h-mayorquin
Copy link
Collaborator

As discussed with @CodyCBakerPhD it seems that the write_imaging pipeline and in particular the add_two_photon_series function is writing our images inverted.

This PR is connected to the proper correction inroiextractors and the requirements consequently pinned to them:
catalystneuro/roiextractors#136

@h-mayorquin h-mayorquin added the high priority currently blocking an issue in another project. That issue should be referenced label Jun 11, 2022
@h-mayorquin h-mayorquin self-assigned this Jun 11, 2022
@h-mayorquin h-mayorquin changed the title correct order of writing in add_two_photon_sequences and pin to branch Correct order of writing in add_two_photon_series Jun 11, 2022
@h-mayorquin h-mayorquin marked this pull request as ready for review June 11, 2022 04:08
@h-mayorquin
Copy link
Collaborator Author

The changes introduce on this were already merged as of #546. We should use this PR to pin the requirements to a specific commit once we merge catalystneuro/roiextractors#136 in roieextractors .

@CodyCBakerPhD
Copy link
Member

Leaving this open until we get a chance to talk about it with Ben...

Be prepared for the possible answer 'NWB ophys (x,y) values actually mean (cols,rows)' lol. We really should just use better language on the NWB schema, so thanks for getting that conversation started.

@h-mayorquin
Copy link
Collaborator Author

Leaving this open until we get a chance to talk about it with Ben...

Be prepared for the possible answer 'NWB ophys (x,y) values actually mean (cols,rows)' lol. We really should just use better language on the NWB schema, so thanks for getting that conversation started.

NeurodataWithoutBorders/nwb-schema#522

@CodyCBakerPhD
Copy link
Member

NeurodataWithoutBorders/nwb-schema#522

Yep, that's what I meant. Thanks for linking.

TBH my own opinion is that on the schema side, it might make the most sense to a new user to refer to their underlying image as 'width' x 'height', since those are quantities they can think about without referencing the data storage (to them it may only appear as an picture on their screen, they may never even interact with it programmatically).

@CodyCBakerPhD
Copy link
Member

Turns out NWB expects width x height, which is the roiextractors equivalent of cols x rows, hence the transposition here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
high priority currently blocking an issue in another project. That issue should be referenced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants