Added support for complex data

[jan-car]

As discussed in #787 I added the two classes for hologram and wave images.
As stated there, HologramImage is nearly empty and WaveImage has some basic functionality (convenience properties and a few functions).
There are a lot of TODOs in the code which in part hold a few things I could still do, for example making the functions work with image stacks. For this I would like to ask what the proper implementation for iterating over the images in the stack in HyperSpy would be. Any ideas?
Other TODOs include names of colleagues of mine (@FWin22 and @woozey), whose tasks it would be (after a successful merge) to fill the two classes with life :-)!
I also added a few basic tests.
I know this PR is in an early state, but maybe you could give me input what could be improved.

---

I'll track the required course of action here:

• Merge Added dependency for scikit-image. #1074 adding the skimage dependency (this should be merged first, because both other PRs implement this as well).
• Merge Let numpy ufuncs operate on HyperSpy BaseSignal instances #1089 for ufunc functionality.
• Merge this PR containing complex functionality.
• Merge Added basic class for hologram images #1079 containing the HologramImage class, testing and documentation, maintained by @woozey. This should be merged last, because it depends on both prior PRs.

[tjof2]

Don't forget the docs! :-) Looking forward to this.

[jan-car]

You're right, I forgot about the docs. I'll try to add as much as I can, again @FWin22 and @woozey, please update the .rst-files in the doc folder when you later add functions :-).
It seems that the checks failed because of skimage could not be found. This package was also used in the .tiff io-plugin, so I am not sure, why it does not work here. Any ideas?

[francisco-dlp]

I've just had a very quick look at this, so take my comments with a pinch of salt.

• Most of the methods in WaveImage are actually far more general, shouldn't they go to a new ComplexSignal class or even (more simply for the moment) to the BaseSignal class?
• All the methods that return a numpy array should return an appropiate hyperspy signals instead.
• To generalize the functionality to n-navigation dimensions simply use the BaseSignal.map function.
• The Image class is now deprecated. We should use Signal2D instead.
• All methods and classes need a docstring.
• It'll be better not to leave comments to your colleagues in the code. Same for TODOs. If the TODO entries are planned for this branch, you could keep you TODO list in the first comment of this PR. If not, you could open a new issue describing what needs to be done.

[jan-car]

Thanks for the feedback!

• The ComplexImage class is definitely a good idea, maybe we can decide which functions should migrate after everything is working?
• Returning HyperSpy signals seems reasonable, is the way I did it with the get_unwrapped_phase method okay (getting a deepcopy and changing the signal type)? I essentially just want to conserve or copy the metadata for the new signals.
• Before the pull request I thought I merged the latest changes to HyperSpy but I still have the old Signal structure. I pulled the "master" branch to my fork but I now see that the new Signal2D and Signal1D classes are in the "0.8.x" fork. Won't it lead to problems, when I just pull this branch and have this pull request point to HyperSpy "master"? (Sorry if this is a dumb question).
• Having the TODOs in a new issue seems much more reasonable (I think this pull request is the wrong place, because they should be done after hopefully merging this PR).
• Concerning the skimage package, I just saw that it is embedded in a try ... except block and there is a HyperSpy internal alternative. Any idea what I should do? The numpy unwrap just operates along one axis as far as I know...

[dnjohnstone]

@CodeMonkeyJan Signal1D/Signal2D went only in to 0.8.x so far and are in PR #1033 to go in to the master. You do need to work off of the master (not 0.8.x) as you have done it's just unfortunate that this came about the same time as the Signal1D/Signal2D merge....

The upshot is that we need to merge #1033 as soon as possible - unfortunately this has been stalled as we were finishing 0.8.5 for a workshop next Tuesday and even more unfortunately more things were added to 0.8.x and master after I made that PR... So now the PR branch needs conflicts resolving and then review.

Feel free to add things to / review that PR though if you want to make this go faster.

[francisco-dlp]

deepcopying inside the unwrapping method seems fine to me. If the numpy function does not operate in ndim, you could also use hspy's map function to generalise it.

Regarding skimage, currently it is an optional package. It probably should be promoted to a required package as some other PRs use functions from them. To add the extra dependency you'll have to edit setup.py, .travis.yml, appveyor.yml, the install section of the user guide and stdeb.cfg (we should provide this instruction in the dev guide...).

[jan-car]

• I included the docs where I thought they would fit. They are a bit short but the idea is that they can be filled later when more functions are added. All functions, properties and classes should be properly documented (class docstrings are missing for other BaseSignal subclasses as well by the way). If I missed a spot for the documentation, just let me know.
• scikit-image should now be a recquired package for hyperspy, the checks all have passed, so I assume it worked.
• All properties and functions now output Image signals as requested. Setting things like amplitude or phase via the properties can now be done with Image signals or numpy arrays.
• All TODOs are removed, @FWin22 and @woozey know what to do from here on.
• Depending on the timing with this and the Signal1D/Signal2D merge into master #1033 PR I can of course change everything to work with the new Signal1D and Signal2D classes. This should be easy refactoring. @dnjohnstone, I'll have a look at Signal1D/Signal2D merge into master #1033, although I'm not sure if I can contribute a lot...
• Now that everything works, what's the general opinion about an intermediate ComplexImage class as @francisco-dlp suggested?

[francisco-dlp]

You could use assert_is instead. The same goes for all other tests in this class.

[jan-car]

Thanks for all the feedback, everything should be implemented now :-).

I added the ComplexSignal1D and ComplexSignal2D classes. I really like this addition, because the structure (see the above image, which should be updated) now allows assign_subclass to fall back to the signal_dimension layer instead of falling all the way to the base ComplexSignal class for complex data with unknown signal_type or signal_origin. We thus have a nice symmetry between real and complex signals.

I left electron_holography.rst in index.rst because it's there where I documented the ElectronWaveImage class. Should we have separate files for this and ElectronHologram later on? I assume the reasoning behind this could be that electron waves have uses in other fields, except electron holography, too, right?

Or something completely different (which is the idea I'm going with at the moment): Now that we have a ComplexSignal2D class, ElectronWaveImage seems a bit superfluous. Currently it has only two functions: add_phase_ramp and plot, which both belong in ComplexSignal2D now. The best idea, in my opinion would be to scrap ElectronWaveImage for now. At a later stage, @FWin22 and others who find they have functions that are not general enough for ComplexSignal2D can add it again (if necessary).

This also means that this PR no longer has anything to do with electron holography and purely adds complex functionality, so I'll change the title accordingly. Oh what a journey it's been :-).

In my personal opinion, adding CommonSignal classes would be more clutter than just copying the functions. If there are more functions which can be shared this could be useful, I think in this case those classes should be abstract and not directly visible to the user then to prohibit them from instantiating "half-baked" classes, which are more like shared interfaces...
Maybe this is something for another PR?

[francisco-dlp]

Great, I think that this is almost ready now. I only disagree with not adding the CommonSignal1D and CommonSignal2D containers for the methods that are common to SignalxD and ComplexSignalxD. I think that I didn't explain myself: I was not proposing to create new BaseSignal subclasses to be included in you diagram. ComplexSignalxD should be only containers e.g.

class CommonSignal1D:
    def to_image

class Signal1D(BaseSignal, CommonSignal1D):

class ComplexSignal1D(ComplexSignal, CommonSignal1D):

The goal is to avoid code duplication.

[jan-car]

Sorry, I think I misunderstood you :-)... Having those classes purely as interfaces seems fine to me (I think that should be the way I implemented them).

[francisco-dlp]

I think that this is ready to merge, do you agree?

[jan-car]

I'd say yes, all tests run fine on my system, but I'd wait until at least one of the builds finishes, because I changed:

if 'complex' in data.dtype.name:

to the (in my opinion) more elegant numpy solution (which I just found today):

if np.issubdtype(data.dtype, complex)

so I did not just change some text in tools.rst. I don't expect any trouble but let's be safe :-).

[jan-car]

Everything should be fine now, from my point everything is ready :-)!

[francisco-dlp]

Happy to merge this awesome contribution.

[jan-car]

Great to hear :-)!
@woozey and @FWin22, I'll assist you with your PRs over the next days.

[francisco-dlp]

@CodeMonkeyJan, it seems that several of the files in the PR contain windows line endings. For any new contribution, could configure your local git as explained here to avoid compatibility issues?

[jan-car]

Oh, I thought I had already done that, apparently I didn't fix that after re-forking HyperSpy, thanks for letting me know!