Fix issue #27 and #16 (Implement getVoxels and setVoxels)

imglib / imglib2-ij

Translation between ImgLib & ImageJ data structures (both 1.x and 2)

Other

4 stars 8 forks source link

Fix issue #27 and #16 (Implement getVoxels and setVoxels) #28

Closed maarzt closed 5 years ago

maarzt commented 5 years ago

This PR fixes issue #27.

That means ImageJVirtualStack now implement getVoxels and setVoxels.

Additionally:

A bug when using AbstractVirtualStack with ColorProcessor is fixed.
Pixel type conversion in ImageJVirtualStack is cleaned up, this fixes #16. Previously ImageJVirtualStack used Converters. Which was very tricky because this only worked for reading but ImageJVirtualStack also supports writing. Now SamplerConverters are used instead.

tpietzsch commented 5 years ago

please format with imglib2 code style

tpietzsch commented 5 years ago

I made some inline comments.

In general, I would really, really, really appreciate it, if you add/fix javadoc while you make changes to the code.

In particular, now that the ImageJVirtualStack sometimes is read-only (swallowing writes) and sometimes write-through, I found it super-difficult to dig back through the layers from the various BdvFunctions.show(...), BdvFunctions.wrap(...) variants to find out in which case what happens. A little bit of documentation would go a long way there.

tpietzsch commented 5 years ago

please format with imglib2 code style

Also make variables final if possible

maarzt commented 5 years ago

@tpietzsch I made all the changes you requested. I force pushed to fix some errors (like typos) before they where made :space_invader: The last five commits are new. First new commit is ImageJVirtualStack: Remove superfluous type parameter.

maarzt commented 5 years ago

@tpietzsch We need to finish this PR rather soon. People stumble across the issues, that are fixed here.