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

N5S3ZarrReader: combine N5ZarrReader logic into N5AmazonS3Reader subclass #5

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshmoore
Copy link

Until there are interfaces, wrappers, or similar composable objects
which allow having both S3 access and Zarr reading, the N5S3ZarrReader
allows a client to explicitly choose the combination of both. This
commit copies methods in order to having a working version of the
reader. Future clean up includes extracting methods to a helper class,
introducing abstract methods to reduce repeated code blocks, and similar.

see: saalfeldlab/n5-aws-s3#10 (comment)

…bclass

Until there are interfaces, wrappers, or similar composable objects
which allow having both S3 access and Zarr reading, the N5S3ZarrReader
allows a client to explicitly choose the combination of both. This
commit copies methods in order to having a working version of the
reader. Future clean up includes extracting methods to a helper class,
introducing abstract methods to reduce repeated code blocks, and similar.

see: saalfeldlab/n5-aws-s3#10 (comment)
@joshmoore joshmoore marked this pull request as draft October 20, 2020 08:00
@joshmoore
Copy link
Author

cc: @axtimwalde @tischi The unit test reading from IDR's S3 is working but the entire data volume has not yet been validated (e.g. viewed) in Java.

@mkitti
Copy link
Contributor

mkitti commented Dec 2, 2020

Following

@tischi
Copy link

tischi commented Dec 2, 2020

FYI,

I temporarily copy and pasted the whole code here: https://github.com/mobie/mobie-viewer-fiji/tree/s3zarr/src/main/java/de/embl/cba/mobie/n5/zarr

Because that it was easier for me to develop code on top to read ome.zarr from s3 into BigDataViewer (BDV).
But since the code is now working I would like to find a new home for it, probably factoring out what is BDV specific, put that somewhere, and then depend on this PR.

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