Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add plot_linescan utility #55

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

jlaehne
Copy link
Contributor

@jlaehne jlaehne commented Mar 5, 2021

Description of the change

Add a function to plot spectral linescans as colormap based on my stock function. Using pcolormesh, it can handle non-uniform axes.

Progress of the PR

  • Change implemented (can be split into several points),
  • update docstring (if appropriate),
  • pass **kwargs to pcolormesh,
  • add tests,
  • ready for review.

Minimal example of the bug fix or the new feature

import lumispy as lum
import numpy as np
s = lum.signals.LumiSpectrum(np.random.rand(10,100)*10)
lum.plot_linescan(s)

@ericpre
Copy link
Contributor

ericpre commented Mar 6, 2021

An alternative would be to add support for non uniform axis to the hs.plot.plot_spectra, wouldn't it be more simple?

@jordiferrero
Copy link
Contributor

An alternative would be to add support for non uniform axis to the hs.plot.plot_spectra, wouldn't it be more simple?

I agree. hs.plot.plot_spectra(s, style='heatmap') can do the same job.

Unless we wanted to add other functionalities to such plots (e.g. markers which follow peaks), I would take @ericpre approach.
I am not sure though of the complexity of hs.plot.plot_spectra. I can try to have a look and see where in the code it fails with non-linear axes...

@jlaehne
Copy link
Contributor Author

jlaehne commented Mar 8, 2021

Hyperspy plot_spectra and other plot functions would need to use pcolormesh to correctly render nonlinear axes. It's on my todo-list to have a look at those. My linescan utility has vertical/horizontal lines, contour lines and interpolation as additional optional features that I don't think are available in hyperspy. But as I had written this function over the years explicitly with non-linear axes in mind, I had never realized that plot_spectra also does linescans. I'll have a closer look at that function again to propose a way to go forward.

@jlaehne
Copy link
Contributor Author

jlaehne commented Mar 9, 2021

plot_spectra from HyperSpy is also missing a figsize argument and uses a non-optimal aspect ratio, which would have to be tweaked to be comparable. At the moment, the cmap argument is also not working.

Adding the parameters the plot_linescan function provides to plot_spectra would overload it in my eyes (even though it might be further improved to use pcolormesh at least).

@jlaehne
Copy link
Contributor Author

jlaehne commented Mar 9, 2021

Example using log-scale, with contours and marker lines:
image

Same data with different figure size, inverted navigation axis, linear scale, different cmap, energy axis:
image

@ericpre
Copy link
Contributor

ericpre commented Mar 10, 2021

plot_spectra from HyperSpy is also missing a figsize argument and uses a non-optimal aspect ratio, which would have to be tweaked to be comparable. At the moment, the cmap argument is also not working.

Adding the parameters the plot_linescan function provides to plot_spectra would overload it in my eyes (even though it might be further improved to use pcolormesh at least).

These should be easy to fixed, the hs.plot.plot_spectra function is fairly simple.
Also it would be good to have the contour functionality as a separate functions acting on matplotlib axes, so that it can be re-use by other functions to plot images, for example.

@jlaehne
Copy link
Contributor Author

jlaehne commented Mar 10, 2021

OK, if you prefer to extend plot_spectra with these extra functionalities, I can integrate the functionalities from here in that function. Adding lines can be an interesting option for spectra as well.

@jordiferrero
Copy link
Contributor

OK, if you prefer to extend plot_spectra with these extra functionalities, I can integrate the functionalities from here in that function. Adding lines can be an interesting option for spectra as well.

@ericpre Did you mean to add these functions in the hyperspy package or here?

I agree with you that the hyperspy plot_spectra is a bit complicated and not particularly useful for fine control... I found myself many times trying to change parameters that could not be accessible with it... and then creating my plotting functions myself and extracting the axes and data manually...

PS:
For the figsize issue, isn't it as simple as plotting with the fig argument (I am not sure but similar things have worked for me before):

f, axs = plt.subplots(figsize=(5,4))
hs.plot.plot_spectra(s, fig=f)

@jlaehne
Copy link
Contributor Author

jlaehne commented Mar 19, 2021

I can have a look to improve plot_spectra to see if I manage to incorporate these things there. Some more versatility would probably be helpful to a lot of people. I also have my own plot_spectra function for some functionalities I was missing in the hyperspy one.

I have kept them separate not to overload a single function, but it is true that a stack/list of single spectra or a linescan do have some things in common and it might not be bad to improve this 'super' function.

Apart from that, we could still decide to have this as a convenience function in LumiSpy. This function is what I have been developing over the years and in itself is quite mature for linescans. Only thing that would need to be added at the moment are the tests.

@ericpre
Copy link
Contributor

ericpre commented Mar 19, 2021

You will be surprise how much functionalities you can pack in a function! ;) hs.plot.plot_images is quite bad but hs.plot.plot_spectra is quite simple and from the discussions here, there are quite a bit of room for improvement.

For example:

import hyperspy.api as hs

s = hs.datasets.artificial_data.get_core_loss_eels_line_scan_signal()
for style in ['overlap', 'cascade', 'mosaic', 'heatmap']:
    hs.plot.plot_spectra(s, style=style)

and this example which I particularly like for its simplicity!

Generally, I find it confusing when several functions/method do very similar kind of things... because it is not clear which functions should be used, the difference, etc... For example, in the last few days, I had to do some interpolation and I find it confusing that there are many functions in scipy.interpolate. Obviously, most of them are different but for some of them, it is difficult to know what are they differences by reading the documentation!

@jordiferrero
Copy link
Contributor

Any updates on this?
Judging from #139 and this PR, it seems that plotting utilities specific for luminescence data always get stuck in PR revision because, in theory, "it can be done with the default hyperspy functions". But doesn't this argument also make the hs.plot functions redundant because matplotlib can do the exact same functionality?

What I am trying to say here @ericpre and @jlaehne is that lumispy could have luminescence-specific plotting utilities (in utils/plot tailored to some key functionalities that are maybe less relevant for EDX or other types of spectroscopic. Maybe some plotting utils make some of the lumispy-demos easier to read/interact with... I don't think that would be terrible.
On the other hand, I do see the downsides of having very similar functions in different packages.

@jlaehne
Copy link
Contributor Author

jlaehne commented Sep 19, 2022

Any updates on this?

I never got around looking at the HyperSpy functions in detail and how to implement the functionality there. Indeed, we should discuss whether we might not have a few specific extra functions here. So far I simply still use my offline version of that function, from which this PR was forked.

It does make sense to give HyperSpy functions some extra functionality, but when they get more complicated also the barrier to use them for specific types of plots gets more difficult. Thinking about it, I do still see an extra benefit of a dedicated linescan plotting function with a very dedicated set of options.

If we should agree on that, I would go over the PR again as some of the arguments could probably be named more close to matplotlib standards, when I originally wrote the code I just used what came to my mind.

@jordiferrero
Copy link
Contributor

Any updates on this?

I never got around looking at the HyperSpy functions in detail and how to implement the functionality there. Indeed, we should discuss whether we might not have a few specific extra functions here. So far I simply still use my offline version of that function, from which this PR was forked.

It does make sense to give HyperSpy functions some extra functionality, but when they get more complicated also the barrier to use them for specific types of plots gets more difficult. Thinking about it, I do still see an extra benefit of a dedicated linescan plotting function with a very dedicated set of options.

If we should agree on that, I would go over the PR again as some of the arguments could probably be named more close to matplotlib standards, when I originally wrote the code I just used what came to my mind.

I would go ahead. But let's wait for @ericpre opinion on having some lumispy-specific plotting utils

@ericpre
Copy link
Contributor

ericpre commented Sep 19, 2022

As far as I understand, the functionalities discussed here are useful for most spectroscopy technique and I don't see what is luminescence specific!

@jlaehne
Copy link
Contributor Author

jlaehne commented Sep 19, 2022

As far as I understand, the functionalities discussed here are useful for most spectroscopy technique and I don't see what is luminescence specific!

Well, for e.g. an EDX linescan, you would normally not plot the full spectra against position (might look fun though). For other signal types used in HyperSpy and not included in LumiSpy it may indeed be useful as well. We simply use linescans quite a lot and this is why such a tool can be handy - in contrast to plot_spectra, where this way of displaying would just be an extra among different functionalities.

@jlaehne jlaehne moved this from In progress to Done in Next release Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants