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

align_zero_loss_peak should take left and right arguments for low loss spectra #3249

Closed

Conversation

HanHsuanWu
Copy link
Contributor

@HanHsuanWu HanHsuanWu commented Oct 29, 2023

Description of the change

Adding additional description for align_zero_loss_peak and added left and right arguments.
When subpixel = True, align1D (with cross-correlation) is used. The range of align1D depends on variables "left" and "right."
Currently by default, left and right are set to be -3. and 3. I think these are in whichever unit the energy axis is in.
Thus, for low loss data that are in the units of meV being about to adjust them will be helpful.

Progress of the PR

  • Change implemented (can be split into several points),
  • update docstring (if appropriate),
  • update user guide (if appropriate),
  • add an changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
  • Check formatting changelog entry in the readthedocs doc build of this PR (link in github checks)
  • add tests,
  • ready for review.

@codecov
Copy link

codecov bot commented Oct 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (70ca076) 81.01% compared to head (6219373) 81.04%.

Additional details and impacted files
@@                  Coverage Diff                   @@
##           RELEASE_next_minor    #3249      +/-   ##
======================================================
+ Coverage               81.01%   81.04%   +0.03%     
======================================================
  Files                     209      209              
  Lines                   32806    32806              
  Branches                 7540     7540              
======================================================
+ Hits                    26579    26589      +10     
+ Misses                   4469     4456      -13     
- Partials                 1758     1761       +3     
Files Coverage Δ
hyperspy/_signals/eels.py 77.58% <100.00%> (ø)

... and 4 files with indirect coverage changes

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

@ericpre
Copy link
Member

ericpre commented Oct 29, 2023

Thanks @HanHsuanWu for the PR, but the EELS/EDS has been split yesterday and these changes now needs to go to https://github.com/hyperspy/exspy.

As you discussed in #3248, currently to use exspy, you will need development branch of hyperspy_gui_traitsui and hyperspy_gui_ipywidgets. In case you don't want to mess around with these, in one work or two, we should have a release candidate of hyperspy and these things should be settled.

@HanHsuanWu
Copy link
Contributor Author

So should I make the pull request in exspy instead?

@ericpre
Copy link
Member

ericpre commented Oct 30, 2023

Yes.

@ericpre
Copy link
Member

ericpre commented Oct 31, 2023

Closing in favour of hyperspy/exspy#7.

@ericpre ericpre closed this Oct 31, 2023
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