-
Notifications
You must be signed in to change notification settings - Fork 5
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
STYLE: Factor out itkOMEZarrNGFFCommon.h #65
base: main
Are you sure you want to change the base?
Conversation
For re-use across ImageIO and TransformIO. OMEZarrNGFFAxis is renamed to OMEZarrAxis for brevity. Also, what was previously ambigiously ome-zarr-ngff is now OME-Zarr.
* | ||
* \ingroup IOOMEZarrNGFF | ||
*/ | ||
struct IOOMEZarrNGFF_EXPORT OMEZarrNGFFAxis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase gone wrong? This was moved to common header in previous commit.
* the "file name", using pattern address.memory, where address is a | ||
* decimal representation of BufferInfo's address. | ||
* Sample filename: "12341234.memory". */ | ||
using BufferInfo = struct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like it should be part of the common class for imageIO and transformIO.
|
||
/** Construct a "magic" file name from the provided bufferInfo. */ | ||
static std::string | ||
MakeMemoryFileName(const BufferInfo & bufferInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also should be common for imageIO and transformIO.
ReadArrayMetadata(std::string path, std::string driver); | ||
|
||
private: | ||
// An empty zip file consists of 22 bytes of "end of central directory" record. More: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also common for imageIO and transformIO
bool | ||
OMEZarrNGFFTransformIO::CanReadFile(const char * filename) | ||
{ | ||
try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe have this function in common class for imageIO and transformIO, and then just inherit it, or at least invoke it, instead of duplicate it?
find_package(ITK 5.0 REQUIRED) | ||
find_package(ITK 5.4 REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upgrade of ITK seems unrelated to/independent of the rest of the commit ("Factor out itkOMEZarrNGFFCommon.h"). If so, preferably make it a separate commit.
For re-use across ImageIO and TransformIO.
OMEZarrNGFFAxis is renamed to OMEZarrAxis for brevity. Also, what was
previously ambigiously ome-zarr-ngff is now OME-Zarr.