Add SIMSSpectrum and FIBSIMSSpectrum signal classes#169
Open
jat255 wants to merge 7 commits intohyperspy:mainfrom
Open
Add SIMSSpectrum and FIBSIMSSpectrum signal classes#169jat255 wants to merge 7 commits intohyperspy:mainfrom
jat255 wants to merge 7 commits intohyperspy:mainfrom
Conversation
- Add SIMSSpectrum for general SIMS data with methods: get_tic, normalize_tic, normalize_to_peak, to_count_rate, get_mass_spectrum, label_peaks - Add FIBSIMSSpectrum (subclass) for FIB-ToF-SIMS depth-profiling with get_depth_profile, integrate_peak, compute_integration_borders - Register both signal types (and lazy variants) in hyperspy_extension.yaml - Update eds.py, eels.py, dielectric_function.py to use public hyperspy.signals API instead of private _signals subpackage - Add tests covering dispatch, normalization, count-rate conversion, and integration border computation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #169 +/- ##
==========================================
- Coverage 88.90% 88.45% -0.46%
==========================================
Files 79 82 +3
Lines 7650 8070 +420
Branches 910 935 +25
==========================================
+ Hits 6801 7138 +337
- Misses 575 656 +81
- Partials 274 276 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
|
This should be ready for a first review. Again, not at all a SIMS expert, so some of these implementations may be off, and if we had someone who has worked with SIMS data that could take a look, that would probably be prudent. |
- Fix normalize_tic(peaks=...) to use np.argmin instead of value2index for robust axis lookup regardless of axis type - Fix get_mass_spectrum(navigation_indices=...) iteration bug: axes from the old signal were passed to the new sliced signal after each sum - Add tests covering: peaks-based TIC, saturation warning, inplace normalization, to_count_rate inplace, get_mass_spectrum both branches, get_depth_profile with explicit mass, and normalize_to_peak inplace - Replace deprecated reader= kwarg with file_format= in TestSignalDispatch - Suppress expected RuntimeWarning in divide-by-zero test - Add pragma no branch for unreachable else in SIMSSpectrum.__init__
- get_tic() promotes spatial (y, x) navigation axes to signal axes, returning a Signal2D per depth slice - plot() defaults to log normalisation for FIB-SIMS data - reintegrate_peaks() re-derives the 4D peak cube from a raw EventList signal, using calibration parameters from original_metadata - Fix normalize_tic() in SIMSSpectrum to sum over the signal axis directly instead of delegating to get_tic(), so subclass overrides don't break normalisation - Update tests: assert Signal2D shape from get_tic(), add TestReintegratePeaks with error and success cases
The rsciio.tofwerk event-list API (compute_peak_data_from_eventlist, _count_active_channels) does not exist in released versions of rosettasciio yet (pending hyperspy/rosettasciio#491). Add a module-level availability check and skip_no_reintegrate marker so the five TestReintegratePeaks success-case tests are skipped in CI until that PR lands.
Member
Author
|
Ok @ericpre this one should be ready for review too. The diff coverage is failing because I had to gate the tests of the "reintegrate peaks" function, since it relies on functionality from hyperspy/rosettasciio#491. Once that PR lands, those tests should run and the test coverage should look good. |
ericpre
reviewed
Apr 11, 2026
Member
ericpre
left a comment
There was a problem hiding this comment.
@jat255, is it okay if we come back to this after hyperspy/rosettasciio#496 is merged and release?
| nbr_waveforms_raw = nbr_waveforms_raw[0] | ||
| nbr_waveforms = int(nbr_waveforms_raw) | ||
| ini = omd.as_dictionary().get("Configuration File Contents", "") | ||
| from rsciio.tofwerk._reconstruction import _count_active_channels |
Member
There was a problem hiding this comment.
If this is needed here, it should be made public in rsciio.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #167
Summary
SIMSSpectrumfor general Secondary Ion Mass Spectrometry data with methods for TIC normalisation, reference-peak normalisation, count-rate conversion, and mass-spectrum extractionFIBSIMSSpectrum(subclass) for FIB-ToF-SIMS depth-profiling datasets, withget_depth_profile,integrate_peak, andcompute_integration_bordersLazySIMSSpectrum,LazyFIBSIMSSpectrum) registered viahyperspy_extension.yamleds.py,eels.py,dielectric_function.pyto use the publichyperspy.signalsAPI instead of the privatehyperspy._signalssubpackage