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

Evaluate tensorstoreToITKComponentType at compile-time, using constexpr #68

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/itkOMEZarrNGFFImageIO.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ OMEZarrNGFFImageIO::PrintSelf(std::ostream & os, Indent indent) const
os << indent << "ChannelIndex: " << m_ChannelIndex << std::endl;
}

IOComponentEnum
constexpr IOComponentEnum
tensorstoreToITKComponentType(const tensorstore::DataType dtype)
{
switch (dtype.id())
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting compile errors from Ubuntu/GCC, https://open.cdash.org/viewBuildError.php?buildid=9845800 at this particular dtype.id() call:

itkOMEZarrNGFFImageIO.cxx: In instantiation of 'constexpr const IOComponentEnum itk::toITKComponentType<float>':
itkOMEZarrNGFFImageIO.cxx:643:3:   required from here
itkOMEZarrNGFFImageIO.cxx:251:84:   in 'constexpr' expansion of 'itk::tensorstoreToITKComponentType(tensorstore::dtype_v<float>.tensorstore::StaticDataType<float>::operator tensorstore::DataType())'
itkOMEZarrNGFFImageIO.cxx:141:19:   in 'constexpr' expansion of 'dtype.tensorstore::DataType::id()'
itkOMEZarrNGFFImageIO.cxx:251:34: error: the value of 'tensorstore::internal_data_type::MakeDataTypeOperations<float>::operations' is not usable in a constant expression

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fixed, by google/tensorstore@91ea2a2 😃

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get #72 and #73 in before we try updating version of tensorstore we use. Or do you want to try that right away Niels?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get #72 and #73 in before we try updating version of tensorstore we use.

Sounds like a plan 👍

Or do you want to try that right away Niels?

No, this one (#68) can wait a little longer, no problem!

Expand Down Expand Up @@ -177,7 +177,7 @@ tensorstoreToITKComponentType(const tensorstore::DataType dtype)
}
}

tensorstore::DataType
constexpr tensorstore::DataType
itkToTensorstoreComponentType(const IOComponentEnum itkComponentType)
{
switch (itkComponentType)
Expand Down