new data transforms: HorizontalFlip, HorizontalRotation, Multiply

[MathGaron]

Ok while the branch is there, maybe we should add a folder (module) with its init and split these in different files.

Also, I know I did not do it for the previous transforms, but while this branch exist, we should add doc string for each transformation giving as much information as possible. Especially about the input output... when I read python code I am always really glad when the author say exactly whats being inputed : ex : np.array of shape [H, W, C] representing a C channel image.. etc...

especially for these kind of functions where we dont really know what is going in and out! I will document my functions :)

[weberhen]

My idea is to keep this branch open as long as we have new data transformations to add (probably forever :) so from time to time we can merge this to develop even if its not in the final version.

[weberhen]

We could maybe include a working example for each transformation. Since each transformation will be in a different file, we can define a main function inside with a call to the transformation, se we have a clear example on how it works and it should serve as documentation too.

[MathGaron]

I dont think it is necessary to write an example for each transformations. They have to be simple input->transform->output so their usage is all the same. I think that simply writting documentation is good enough ex:

bb_point2rect : tranforma a boundingbox from points to rect form ... input : np.array boundingbox of the form [x1, y1, x2, y2] with shape [N, 4] output : np.array boundingbox of the form [x, y, w, h] with shape [N, 4]

also I am not sure why you added the gaussian mixture model in there? I dont see how it fits in transformations?

[weberhen]

yeah, it was not the best idea to add gmm to transformations, so I just remove it from there. Also I split all classes in their own files, so @MathGaron please add docstring to the transformations you created (I'll do that too) when you have time ;)

[MathGaron]

I will for the doc. A question though : I am not sure but for the transforms I thought that we could split them by groups and not per transforms. Let me give an example :

boundingbox (all bb related transforms) depth (all depth channel related transforms) image ( all image (rgb) related transforms) hdr (all hdr related transforms)

what do you think?

[weberhen]

Yeah I was doing that but I saw two potential problems: 1) unnecessary dependencies being imported: Lets say we have the HDR module. For now, I need opencv to perform tonemapping, but for other operations (horizontal flip) I dont need it. So if I want to use horizontal flip I would need to import cv2. 2) general functions: I created horizontal flip, which can be used for hdr or ldr images. So where should we put this function? In the end I think we will lose time trying to categorize transformations and after having to open each file and search for the function we want.

Do you see other advantages from grouping functions other than having less files lying around? If yes let's discuss :D

[MathGaron]

Mostly to know where to look when we need something. For exemple if I want to do data augmentation on Images, and I dont really know what I want, I could just open the image file and scroll them. Else I need to check every file to know on what kind of data they work.

For categories I would try to put them in the most general file. For exemple we have Image, so the transform have to work in anything that is defined as an image (height, width, channel). HDR is a special kind of image, and most people doing CV wont need it, and these transform are only useful for an HDR image.

another example is the panorama rotate, it should not be in image as the rotate operation is not defined for general images, but it can be in a panorama category.

We have to think about how we document also, do we want to give a list of all possible operation over all kind of data, or subdivide this in another documentation page, or make the code document itslef. In my opinion the latest is the best.. you want a transform, go in data transform folder and see that :

• Image
• Bounding Box
• HDR
• Panorama
• Image of turtle
• String
• Donald Trump quotes
• [...]

[weberhen]

Good point. Just the last problem remains: I have a fancy data transformation defined for images that use an obscure python library. I will put this transformation together with the other ones and therefore will force everyone to import this library. How do we deal with that?

[MathGaron]

Yeah this is a good point. I think we should treat these special cases independently... for exemple, if the transform is so unusual, I dont think we should put it here, the user will define it in its own library and thats all. If the transform is really general, and may be usefull in most cases then we need to think if its worth adding the dependency for the whole package.

You have some examples of this case in mind?

[MathGaron]

The key point here, is the most general module (Image for example) should have general transforms.. flips.. crops etc (same as torchvision module in fact) If there is some special case like Image of turtle and transform related to that only, you make another module for it and import your weird_turtle_module.

But even there, maybe that turtle thing has nothing to do here.. it's way too specific..

[weberhen]

My example for now is opencv for hdr operations... for me its fine, and if some point @jacenfox is using a new transformation I would like to see it in the toolbox even if its unorthodox :P

We have 3 options: 1) we forbid fancy transformations (i.e require weird libs) 2) we allow for lazy import (put weird lib import inside function so it will be called only if the function is needed) 3) divide all functions in their own file

For me now it seems that maybe number 2 is a good option, since its not a bad practice (if you want we can add a comment at the top of the file specifying the lazy imports) and helps to keep the files organized.

[MathGaron]

Yes I don't see anything against lazy import. You could call it in the init of the transform object. Or you could also make it dependent on the whole HDR module. Lazy importation is probably more flexible though.

[MathGaron]

@weberhen can you please write the doc strings so we can close this pull request? I will finish up the stuff on my side! this request has been up for too long hehe

[weberhen]

done!