Open gcerutti opened 7 years ago
Hi, First, thanks for your positive feedback. Several points may be mentioned about this issue:
Dear all, Yes, we have a/several problem(s).
I think that such service, plugin, adaptors need to be manage somewhere. I recommend a kind of generic/abstract (api) module that was first in openalea.image.
These are very good points, and i don't think the situation is desperate! :) Only that there is a bit of ambiguity to clarify, otherwise it may be prejudicial to the user.
If i'm right, we could consider openalea.image.SpatialImage and timagetk.SpatialImage as two not-so-distinct-yet-different implementations of the "Image" interface (as defined in openalea.image)? If they are to be considered different, i just think using different class names would be enough to remove the ambiguity (especially since they can easily be "cast" into each other via their new methods, provided you pass some information such as the voxelsize).
So yeah, this would not be such a big problem, as @sribes says, all the timagetk examples work in the TissueLab application for instance, and if you have a more complex scenario that involves openalea.image.SpatialImage, just cast them into timagetk.SpatialImage before anything! This is what i do every time i use scipy or skimage to process an image, and have to cast the numpy.ndarray result to an openalea.image.SpatialImage anyway. It just requires to make clearer that they are different objects.
But if we consider both classes essentially do the same thing (implementing the abstract Image data type) and are used in the same development environment (with the same name) there should be a way of tying them together so that the user/application could use them indifferently and in a transparent way. And i agree with @sribes that these links should be implemented without affecting in any way the autonomy of the timagetk package.
If, according to @pradal, the transform has to be handled by services, one question is where should we define the adapters? (such as mimetypes codecs? 'timagetk/interface.IImage' <==> 'openalea/interface.IImage') Is it up to timagetk to define an openalea codec/adapter plugin for the conversion (in both ways?) as it would be designed as "potentially interacting with an openalea ecosystem"? Or would it be preferable to define codecs in openalea.image, even if that means modifying this repo every time a new openaela-related package is added?
Or could an even more drastic solution be implemented? Hard-coding the fact that timagetk provides an enhanced version of the openalea.image library, and therefore test, in openalea.image, if the basics (SpatialImage, imread, imsave) can be imported from timagetk, and if so simply overwrite the openalea.image ones? I really like the service/adapter solution, but i fail to see how the dynamic conversion would occur in calls to algorithms from python code for instance... So such a shortcut might be a satisfying solution, what do you think? Or have you had the time to have a look at the "solution" i proposed in #8?
In my opinion, an update of the folder openalea.image or an update of the folder timagetk.components is not the good way and can have strong negative repercussions for algorithms and for users. This modification concerns a data structure and io components, and can introduce a lot of bugs. If a modification has to be done, it has to be accompanied by non-regression tests, documentation, etc.
For me, the modification should be done in OpenAleaLab or in TissueLab, since these developments aims to propose abstractions, interfaces, services, contracts. The visualization problem is related to the fact that the function depends on a specific implementation instead of passing through a service.
Can someone move this issue to OpenAleaLab or TissueLab ?
I think it would be desireable that, in the cases where an existing openalea installation is already used in the environment, the SpatialImage class defined in timagetk does not act as a duplication, that may lead to hardly understandable situations.
I'll take a quick example : i am using TissueLab to load and visualize tissue images, typically using the drag and drop mechanism that would natively use the
imread
function fromopenalea.image
to load an image file. Now, say i want to use timagetk to applylinear_filtering
to one of these images in the application (which should be a reasonable wish) i'll get an error because the object is "not an instance of SpatialImage"! But yes, of course, it is!! (okay, not the same) This kind of behaviour will be really confusing for a typical user.I'd suggest two possible solutions :
TissueImage
as basic class and everybody is aware of the difference.openalea.image
in the case it is installed (in a way that is transparent to the user) without losing the (great) self-contained aspect of the timagetk package.