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

Transpose centroid output #184

Merged
merged 2 commits into from
Mar 17, 2023
Merged

Transpose centroid output #184

merged 2 commits into from
Mar 17, 2023

Conversation

jlaehne
Copy link
Contributor

@jlaehne jlaehne commented Mar 17, 2023

Description of the change

I realized that for a map the new centroid function, similar to e.g. valuemax and indexmax in HyperSpy returns a BaseSignal with 2 navigation dimensions and 0D signal, which theoretically makes sense, but is inconvenient for plotting as it plots including the navigator, which makes little sense for 0D signals, and colormap cannot be changed.

In contrast, estimate_peak_width returns the transposed signal, thus Signal2D object without navigation dimensions, which is more convenient for plotting - and is okay in the sense that it is just an image of values for the different positions. The as_signal command for parameters from a fit also results in Signal2D. I therefore think, it is the more sensible output here.

We should consider making that more consistent also in HyperSpy (e.g. with the 2.0 release).

As I was at it, I also removed optional parameters from the title if they are not set.

Progress of the PR

  • Change implemented (can be split into several points),
  • docstring updated (if appropriate),
  • update user guide (if appropriate),
  • added tests,
  • ready for review.

Minimal example of the bug fix or the new feature

from lumispy.signals.luminescence_spectrum import LumiSpectrum
s = LumiSpectrum([[[1, 2, 3, 4, 5]] * 3] *4)
com = s.centroid()
com

now results in

<Signal2D, title: Centroid map, dimensions: (|3, 4)>

@jlaehne jlaehne added this to the v0.2.2 milestone Mar 17, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.55 ⚠️

Comparison is base (d3cf6ac) 100.00% compared to head (014c93b) 99.45%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff             @@
##              main     #184      +/-   ##
===========================================
- Coverage   100.00%   99.45%   -0.55%     
===========================================
  Files           12       12              
  Lines          545      554       +9     
===========================================
+ Hits           545      551       +6     
- Misses           0        3       +3     
Impacted Files Coverage Δ
lumispy/signals/luminescence_spectrum.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jlaehne jlaehne mentioned this pull request Mar 17, 2023
3 tasks
@jlaehne jlaehne force-pushed the centroid-2dsignal branch 4 times, most recently from e744c18 to 014c93b Compare March 17, 2023 22:49
@jlaehne jlaehne merged commit 9db99bd into LumiSpy:main Mar 17, 2023
@jlaehne jlaehne deleted the centroid-2dsignal branch December 22, 2023 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants