Page MenuHomePhabricator

Design Decision: Interface of MITK Python Wrapping
Closed, ResolvedPublic

Description

We now have a working prototype of an Wrapping of MITK for Python. It is possible to build packages for Windows and Linux and it is possible to convert images from and to numpy array, the default matrix element for python.

We now need to discuss the interface for the wrapped code:

  1. Which classes should be wrapped in python?
  2. Which Methods should be wrapped?
  3. How to handle different modules?

Event Timeline

goetzm triaged this task as Wishlist priority.Feb 27 2018, 5:02 PM
goetzm created this task.

To 2:
By default, the whole class structure is exposed in the wrapped language, e.g. right now the user is able to call all methods of each wrapped class, for example mitk::Image. During the wrapping process it is also possible to either whitelist or blacklist the methods that should be exposed. Also a renaming of the methods is possible. Further, an extension of a class with custom c++ code or code in the target language is possible.

To 1:
The most common basic datatypes (std::string, double, int etc..) are wrapped. Also, typical std::maps and std::vector variants (like std::vector<std::string> ) are wrapped. This allows to pass these datatypes to mitk code and to read them if they are returned by mitk code.

The following mitk classes are explicitly wrapped:
IOUtil,
mitk::BaseData
mitk::SlicedImage
mitk::Image
mitk::PointSet

In order to have the automatic casting working as expected, it is necessary to wrap all datatypes in the depth of inheritance. This is the reason that mitk::SlicedImage is wrapped right now. RIght now, std::vectors of the wrapped mitk classes are also wrapped.

It is possible to have classes only partially wrapped. Right now, this is done so for mitk::PixelType and mitk::ChannelDescriptor. It is then possible to interact with such elements if they are returned by an method, but the creation / casting is limited.

The classes that are going to be wrapped needs to be whitelisted by selecting the corresponding includes.

To 3:
Right now, a single package is create for the target language (So far, only python). This is somewhat contrary to the modular system of mitk.

SWIG, which we use for the wrapping, has only a limited support for different modules. So in order to create multiple modules, some CMake-Magic needs to be done. Of course the same is true if we want to have the possibility to turn modules on/off. ...

So my opinion to 1:

  • QT is really difficult to ship, especially for linux. I would therefore strongly advocate agains having any dependency to it in the library.
  • I will definitively include the brand-new Mitk Phenotying code, as soon as i merged it into the master. This is the main reason for me to create the wrapping.
  1. How maintenance intensive is the wrapping going to be? The more custom the solution and the more hand-written code it contains the more of a nightmare I assume it to be if the inevitable interface change of some central class arrives.
  2. Is there a way to automatically test this via a python dart client? Do you already have something like that?
  3. If I understand correctly we "only" have to wrap parent classes of the classes we want to use in python?

About Goch 1:
Judging from my experience now, i think that maintaining the code is rather straight forward. The main wrapping is done by including the corresponding header files, there is nearly no custom code that is writtern for an individual class. So i think that it won't make a big difference in terms of maintainance if 5 or 500 filters are wrapped.
This changes if custom additions are made to a class. For example, the interface mitk::Image <-> Numpy needs some maintainance.

About Goch 2:
Yes, there are ways to do this, although I didn't looked at them yet. One idea would be to set-up two docker images. The first one is used to build the python install files (e.g. wheels) and the second one is used to install these wheels and run some basic test scripts. We could setup this infastructure by our own or use existing solutions, for example travis-ci (https://travis-ci.org/)

About Goch 3:
No. Each class needs to be "wrapped", separatly. There are two different "facts":

  1. If we want to be able to use the inheritance structure of a class, all relevant sub-classes etc.. must be wrapped. An example: In order to be able to cast from mitk::BaseData to mitk::Image automatically, we also need to wrap mitk::SlicedImage. This allows us to call a method that requires a BaseData object with an Image. (which would also be possible in C++ using casting). If this isn't needed, there is no need to have the parent class wrapped to. But i would strongly argue that it makes sense to keep the inheritance structure for all classes derived from BaseData.
  2. A class is automatically wrapped, if the class description is given in the SWIG configuration files. This can either be done by writing the class description directly into the configuration file (Bascially a c++ class definition), which allows to include only those details that we want to have wrapped (whitelist). Or to include the corresponding header file, which by default wraps the complete (public) class. It is then possible to hide specific methods or members of the class with SWIG-commands, (whitelisting). But each class that is wrapped must be either specified in the included headers or in the config files.

Sorry to be a nitpicker, but regarding 3 (and as I can find no reference to SlicedImage anywhere in the code or phabricator, apart from this task):

Assuming we have the following inheritance structure:

BaseData
|
SlicedData
|
Image
|
PeakImage

If I want to enable casting from BaseData to Image I only have to wrap BaseData, SlicedData and Image, correct? If PeakImage also needs to be wrapped (which is how I read your last answer) the whole wrapping would collapse any time some third party added a new derived data type without telling us about it.

Does it depend on whether or not the BaseData-to-be-casted is in fact a PeakImage we just happen to only want to cast to Image?

Sorry, I meant SlicedData , not SlicedImage of course. This might be a source of missunderstandings.

Back to your question:

  • Yes, to enable casting from BaseData to Image you only have to wrap BaseData, SlicedData and Image.
  • PeakImage does not necessarly need to be wrapped. Only, if you need the functionality of PeakImage
    • Without Wrapping PeakImage:
      • Assumed that the loader available, it is possible to load a PeakImage as a BaseData Object.
      • It would be possible to Cast a PeakImage-BaseData-Object to an Image or vice versa
      • The handling would be like that of any other image
      • It would NOT be possible to cast to PeakImage
    • Wrapping would be needed if
      • A function of PeakImage is needed
      • Casting to/from PeakImage is needed

So, an example from our code: We do have LabelImage, a class derived from Image. It is currently not wrapped but loading, saving and casting such images is perfectly possible.

Did this help to clarify your question?

Any thoughts so far? Should we set-up a discussion? Perhaps this friday?

Friday would be ok from my side. Due to community day I am very flexible.