ENH: Add explicit Matrix(const RawDataArrayType &) constructor

[N-Dekker]

Allows constructing an itk::Matrix by passing a C-style array of
arrays, of the corresponding value type and dimensions.

[N-Dekker]

Passed well on the regular C++ builds (ITK.Linux, ITK.Windows, ITK.macOS), but failed on the Python builds, saying:

Wrapping/Typedefs/itkMatrix.i:76: Error: Syntax error in input(3).

At https://open.cdash.org/viewBuildError.php?buildid=7798097

Any clue? Would using RawDataArrayType = T[VRows][VColumns] be a problem to SWIG? Would it prefer an old-style typedef for array types?

[dzenanz]

T[VRows][VColumns] a problem to SWIG

Probably. @thewtex will know, maybe even @PranjalSahu (he had a close encounter with Wrapping recently).

[N-Dekker]

This line of code:

ITK/Modules/Core/Common/include/itkMatrix.h

Line 69 in d9a729c

using RawDataArrayType = T[VRows][VColumns];

using RawDataArrayType = T[VRows][VColumns]; 

Could be rewritten to "old-style":

typedef T RawDataArrayType[VRows][VColumns]; 

Just wondering if that would fix the SWIG errors...

[dzenanz]

I don't think old-style typedef would make any difference compared to using. I think that the problem comes from the actual type attempting to be wrapped.

[N-Dekker]

My most recent force-push removed the typedef (using alias) for T[VRows][VColumns], but the SWIG errors are still there 🤔

At https://open.cdash.org/viewBuildError.php?buildid=7799641

Wrapping/Typedefs/itkMatrix.i:76: Error: Syntax error in input(3)

So I guess SWIG just does not like a reference to a C-style array as parameter (like the one of the proposed new explicit itk::Matrix constructor)... but I'm not sure. 🤷

I still believe the pull request would be useful, but apparently it doesn't "swig".

[N-Dekker]

@dzenanz Interesting... it looks like my last force-push has worked around the SWIG errors!!! 😄 SWIG appears to like:

template <typename TRow>
explicit Matrix(const TRow (&rows)[VRows])

While SWIG did not like:

explicit Matrix(const T (&arrayOfArrays)[VRows][VColumns])

Personally I would prefer the const T (&arrayOfArrays)[VRows][VColumns] version, but eventually I guess it's up to SWIG what to do 🤔 !

[Leengit]

SWIG didn't like the non-templated code, but in this case there are multiple ways to template. I don't know that they are any better, but for SWIG research purposes, did you happen to try either of

  template <typename TMatrix>
  explicit Matrix(const TMatrix & matrix)
    : m_Matrix(&matrix[0][0])
  {
    static_assert(std::is_same<TMatrix, T[VRows][VColumns]>::value,
                  "The type of the matrix should correspond with this itk::Matrix instantiation.");
  }

  template <typename TElement>
  explicit Matrix(const TElement (&elements)[VRows][VColumns])
    : m_Matrix(&elements[0][0])
  {
    static_assert(std::is_same<TElement, T>::value,
                  "The type of an element should correspond with this itk::Matrix instantiation.");
  }

[N-Dekker]

SWIG didn't like the non-templated code, but in this case there are multiple ways to template. I don't know that they are any better, but for SWIG research purposes, did you happen to try either

No I did not. Thanks for the suggestion @Leengit. Matrix(const TMatrix & ) does not really appeal to me, because it might be too greedy, in case of overload resolution. Although I guess you might SFINAE away non-array-of-array argument types. Matrix(const TElement (&elements)[VRows][VColumns]) would certainly be better, but I'm afraid SWIG wouldn't like it. Although I'm not sure. So you might check, if you really want to know.

[Leengit]

Thanks @N-Dekker. Your code is a-okay, so there is no need to put additional work into these variations. I was asking in case you already had the information.

[N-Dekker]

@Leengit Still trying your last suggestion (const TElement (&elements)[VRows][VColumns]), with this force-push... will it SWIG or not?

Update: YES it SWIGs!!!