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

plot_spectra heatmap should take kwarg as well #3219

Merged
merged 4 commits into from Sep 5, 2023
Merged

plot_spectra heatmap should take kwarg as well #3219

merged 4 commits into from Sep 5, 2023

Conversation

HanHsuanWu
Copy link
Contributor

As title, currently plot_spectra heatmap does not take kwarg for .plot().

Copy link
Member

@ericpre ericpre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstrings on **kwargs needs to be updated, for now, it says "Has no effect on 'heatmap' style."
It also needs a changelog entry - this is useful to inform user of change of behaviour but also for developer to keep track when changes have implemented.

Can you rebase the changes on the RELEASE_next_major branch, because we are aiming at releasing hyperspy 2.0 mid-Sept. and working on that branch will avoid us to merge the RELEASE_next_minor into the RELEASE_next_major.

@HanHsuanWu HanHsuanWu changed the base branch from RELEASE_next_minor to RELEASE_next_major September 2, 2023 17:32
@HanHsuanWu
Copy link
Contributor Author

Hi Eric,

I change the doc for plot_spectra and added a changelog entry.
I have rebased to next_major and changed the commit to next_major.

@codecov
Copy link

codecov bot commented Sep 2, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (c0f06a3) 81.31% compared to head (1e3f129) 81.31%.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           RELEASE_next_major    #3219   +/-   ##
===================================================
  Coverage               81.31%   81.31%           
===================================================
  Files                     173      173           
  Lines                   24206    24206           
  Branches                 5623     5623           
===================================================
  Hits                    19683    19683           
  Misses                   3223     3223           
  Partials                 1300     1300           
Files Changed Coverage Δ
hyperspy/drawing/utils.py 76.56% <100.00%> (ø)

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

Copy link
Member

@ericpre ericpre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested clarifications/more details for the docstring.

hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
HanHsuanWu and others added 2 commits September 4, 2023 11:34
Co-authored-by: Eric Prestat <eric.prestat@gmail.com>
@ericpre ericpre merged commit fa7a7e0 into hyperspy:RELEASE_next_major Sep 5, 2023
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants