Axis conversion for TransientSpec

[jlaehne]

Description of the change

Move axis conversion code to CommonLuminescence class to make it available to LuminescenceTransientSpectrum signals.

Handle inheritance for slicing of LuminescenceTransientSpectrum:

• Create 1D version of signal_type = 'TransientSpec' (class TransientSpectrumCasting)
• If TransientSpec is initialized in 1D with axes_manager[-1].units being a time unit, switch to LumiTransient class, otherwise switch to Luminescence class

Progress of the PR

• move axis conversion code,
• Implement workaround for Set axis replaces wrong axis for signal dimension > 1 hyperspy/hyperspy#3294
• Handle slicing of TransientSpec,
• add section to user guide,
• add tests for TransientSpec case,
• added line to CHANGELOG.rst,
• check formatting of changelog and user guid,
• ready for review.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 97.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 99.29%. Comparing base (9c402c0) to head (2a354b6).
Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
lumispy/signals/common_luminescence.py	97.56%	2 Missing ⚠️
lumispy/signals/luminescence_transient.py	50.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##              main     #205      +/-   ##
===========================================
- Coverage   100.00%   99.29%   -0.71%     
===========================================
  Files           12       12              
  Lines          556      567      +11     
===========================================
+ Hits           556      563       +7     
- Misses           0        4       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ericpre]

I don't like the idea of introducing the concept of "hidden signal" because by design registered signal class are not hidden.

If I understand correctly, the purpose of the TransientSpectrumCasting class is to assign to the correct class depending on the units of the last axis when converting to 1D signal. If so would it make sense to move this code to LumiTransient.__init__ and change to the Luminescence signal type when necessary? It should work if the TransientSpec signal type was added as an alias of the LumiTransient class?

[jlaehne]

@ericpre I was also hesitating to introduce a 'hidden signal class', but it works like a charm - while I cannot make it work without.

It is more elegant than what I proposed in #204 (comment) (and did not work), so I proposed this solution in #204 (comment)

I tried again having the initialization in the __init__ of one of the two existing 1D classes, but it then does not properly convert to the other. Instead, I have improved the documentation as you proposed.

[jlaehne]

@ericpre do you have an idea why the changelog build displays a numba warning? See: https://lumispy--205.org.readthedocs.build/en/205/changelog.html
Do I have to add that dependency to the workflow now that it is optional upstream?

Also, any idea on the Azure Pipelines failures?

[ericpre]

@ericpre do you have an idea why the changelog build displays a numba warning? See: https://lumispy--205.org.readthedocs.build/en/205/changelog.html Do I have to add that dependency to the workflow now that it is optional upstream?

This is surprising that it appears there! Yes, adding numba as dependencies or using hyperspy[speed] can be used a workaround.

Also, any idea on the Azure Pipelines failures?

mambaforge is deprecated, I will update what needs to be updated later today! 😉

[jlaehne]

@ericpre Do you want to have another look.

[ericpre]

This looks good, I have some comments to improve the documentation.

import lumispy as lum
import numpy as np

data = np.arange(10*20).reshape(10, 20)

s = lum.signals.LumiTransientSpectrum(data)
s.axes_manager[-1].units = "ns"
s.axes_manager[-2].units = "nm"

s.sum(-1)
s.sum(-2)

[ericpre]

Is there code in lumispy that can make this figure? When I do lumispy.signals.luminescence_transientspec.LumiTransientSpectrum.plot, I get a normal plot.

It would be good to add the code to make this figure, otherwise it should be removed?

[jlaehne]

I had previously prepared this figure for illustration purposes and thought it could be useful here to show the principle. It is indeed a useful feature to generally faciliate such kind of plots and I will open an issue to implement that in a separate PR (which could probably be taken on by one of my students). Not as the standard plotting of this class, but as a separate plot function (alternative to using sum an option to display spectra/transients for a certain range (or even an interactive span functionality can then be added later).

[ericpre]

Am I understanding correctly that this is simply moved around and doesn't need reviewed of the code itself?

[jlaehne]

Yes, all the code pertaining to signal axis conversion is moved to the CommonLuminescence class so that it is available also to LumiTransientSpectrum. That was the origin of this PR and I probably should have merged that moving part in before doing the other changes to make it clearer.

[jlaehne]

This looks good, I have some comments to improve the documentation.

I implemented your suggestions (and extended some to the rest of the documentation). Only the plot creation will need a separate PR.