hyperspy
hyperspy copied to clipboard
Add plot display range
Description of the change
Improve setting x limits when plotting EDS signal, which can happen in cases, as the one described in #2899. Since this is not the first time that this question is asked and this is simple issue, it would be good to make it more convenient.
Progress of the PR
- [x] Display EDS spectrum from 0.1 keV,
- [x] update docstring (if appropriate),
- [n/a] update user guide (if appropriate),
- [x] add an changelog entry in the
upcoming_changesfolder (seeupcoming_changes/README.rst), - [x] Check formatting changelog entry in the
readthedocsdoc build of this PR (link in github checks) - [x] add tests,
- [x] ready for review.
Minimal example of the bug fix or the new feature
From the example given in #2899
import hyperspy.api as hs
# For Signal1D
s = hs.load('2030 8.00 Mx 0001.emd')[-1]
s.sum().plot()
# For Signal2D
ima = hs.datasets.example_signals.object_hologram()
ima.plot(display_range=(150, 400, 200, 450))
Before this PR:

With this PR:

Codecov Report
Merging #2958 (5790878) into RELEASE_next_patch (8d37cc1) will increase coverage by
0.01%. The diff coverage is95.34%.
@@ Coverage Diff @@
## RELEASE_next_patch #2958 +/- ##
======================================================
+ Coverage 80.79% 80.80% +0.01%
======================================================
Files 209 209
Lines 32696 32726 +30
Branches 7321 7336 +15
======================================================
+ Hits 26416 26444 +28
- Misses 4516 4517 +1
- Partials 1764 1765 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| hyperspy/docstrings/plot.py | 100.00% <ø> (ø) |
|
| hyperspy/drawing/signal1d.py | 80.61% <93.75%> (+0.55%) |
:arrow_up: |
| hyperspy/_signals/eds.py | 85.71% <100.00%> (+0.15%) |
:arrow_up: |
| hyperspy/drawing/image.py | 74.30% <100.00%> (+0.36%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 8d37cc1...5790878. Read the comment docs.
I am not sure if it is a good idea to adjust the x-limit to 0.1 keV automatically—this hides a feature of the data from the user, what can cause other issues. Wouldn't it be better to simply document this particularity of EDX datasets and suggest to crop the data using s.isig[0.1:] as a cure?
Also, this looks more like an "enhancement" than a bug-fix to me. Do you agree @ericpre?
I am not sure if it is a good idea to adjust the x-limit to 0.1 keV automatically—this hides a feature of the data from the user, what can cause other issues. Wouldn't it be better to simply document this particularity of EDX datasets and suggest to crop the data using
s.isig[0.1:]as a cure?
I was torn on this for a while and was thinking the same, however, I end up convincing myself that there is a use case to have this:
- the issue with high intensity of zero energy peak in EDS spectrum is mentioned once in a while by users. As you said, this can be work around by using
isig, but it come at the cost of making a copy of the data, which is not always an option. Assuming that 99% of the user aren't interested in the zero energy peak, it is useful to have it as default. The other 1% would be advanced EDS user and spend effort looking at the data can afford changing the default setting to their preference. - for
Signall2D, I find it useful to be able to specify the range of displayed data without having to make a copy. An alternative to this would be use matplotlib differently, but there is currently a bug with the scalebar and it is more verbose.
Overall, because it is generic and the logic is simple (~I can't think of any issue that it could cause and I don't expect any because it is simple logic~, EDIT: clearly this statement didn't hold very long!! :D See https://github.com/hyperspy/hyperspy/pull/2958#discussion_r898827390), I came to the conclusion that this is a good compromise. The EDS plotting uses specific default values, which is not documented correctly.
Also, this looks more like an "enhancement" than a bug-fix to me. Do you agree @ericpre?
Yes, it is mostly an enhancement, on the edge being a bug! ;)
isig doesn't make a copy of the data, just a view, so it shouldn't be a problem to use it/
The issue I see is that the offending channel is also an issue for decomposition and curve fitting. By taking it out of the view, we hide the issue, making it harder for the users to understand what's wrong in their analysis.