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.

Sign up for GitHub

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

Jump to bottom

[Enhancement I] Add multichannel volume #13

Merged
merged 21 commits into from
Jul 15, 2024
Merged

[Enhancement I] Add multichannel volume #13

merged 21 commits into from
Jul 15, 2024

Conversation

CodyCBakerPhD
Copy link
Member

This pretty much completes the absorption of relevant parts of ndx-multichannel-volume into our microscopy metadata structure, with the exception we want to avoid of the 5D time + volume + channels object, which we can include in a follow-up if desired

@CodyCBakerPhD CodyCBakerPhD self-assigned this Jun 24, 2024
@CodyCBakerPhD CodyCBakerPhD changed the title Add multichannel volume [Enhacement II] Add multichannel volume Jun 27, 2024
@CodyCBakerPhD CodyCBakerPhD changed the title [Enhacement II] Add multichannel volume [Enhacement I] Add multichannel volume Jun 27, 2024
@CodyCBakerPhD CodyCBakerPhD mentioned this pull request Jun 27, 2024
Merged
@CodyCBakerPhD
Copy link
Member Author

@rly WDYT? Good to merge to get this chain moving?

@CodyCBakerPhD CodyCBakerPhD requested a review from rly July 11, 2024 18:17
@CodyCBakerPhD
Copy link
Member Author

@alessandratrapani Any thoughts?

@CodyCBakerPhD CodyCBakerPhD changed the title [Enhacement I] Add multichannel volume [Enhancement I] Add multichannel volume Jul 12, 2024
@rly
Copy link
Contributor

rly commented Jul 13, 2024

Looks good to me. I suggested adding dims and shape to light_sources and optical_channels to enforce the 1D list nature of the underlying VectorData which can be multidimensional.

@CodyCBakerPhD @rly
Apply suggestions from code review
b05a3e6 
Co-authored-by: Ryan Ly <rly@lbl.gov>
@CodyCBakerPhD CodyCBakerPhD mentioned this pull request Jul 15, 2024
Merged
@CodyCBakerPhD
fix outer spec to match altered one (#20)
b476136 
Co-authored-by: CodyCBakerPhD <codycbakerphd@gmail.com>
@CodyCBakerPhD
Copy link
Member Author

@rly OK, added dim/shape

@CodyCBakerPhD CodyCBakerPhD requested a review from rly July 15, 2024 17:29
@CodyCBakerPhD CodyCBakerPhD merged commit e5c3892 into main Jul 15, 2024
33 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the add_multichannel_volume branch July 15, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rly rly rly approved these changes

@alessandratrapani alessandratrapani Awaiting requested review from alessandratrapani

Assignees

@CodyCBakerPhD CodyCBakerPhD

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CodyCBakerPhD @rly