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

Implement plot span map #3224

Merged

Conversation

0Hughman0
Copy link
Contributor

@0Hughman0 0Hughman0 commented Sep 6, 2023

Note this is a port of this PR to lumispy

Description of the change

A few sentences and/or a bulleted list to describe and motivate the change:

  • implemented plot_span_map. This is a tool for automatically plotting a spatial map of intensity of a hyperspectral map over a number of ranges defined by a SpanROI.

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.

Minimal example of the bug fix or the new feature

import numpy as np
import hyperspy.api as hs

s = hs.signals.Signal1D(data=np.random.random((64, 64, 1024)), 
                        axes=[{'name': 'x', 'size': 64}, 
                              {'name': 'y', 'size': 64}, 
                              {'name': 'sig', 'size': 1024}])

hs.plot.plot_roi_map(s, rois=2)

Capture

# Copyright 2007-2023 The HyperSpy developers
#
# This file is part of HyperSpy.
#
# HyperSpy is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# HyperSpy is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with HyperSpy. If not, see <https://www.gnu.org/licenses/#GPL>.

"""Plotting funtions.

Functions:

plot_images
    Plot multiple images in the same figure.
plot_spectra
    Plot multiple spectra in the same figure.
plot_signals
    Plot multiple signals at the same time.
plot_histograms
    Compute and plot the histograms of multiple signals in the same figure.

The :mod:`~.api.plot` module contains the following submodules:

:mod:`~.api.markers`
    Markers that can be added to :py:class:`~.signal.BaseSignal` figure.

"""

from hyperspy.drawing.utils import (
    plot_histograms,
    plot_images,
    plot_signals,
    plot_spectra
    )
from hyperspy.utils import markers

__all__ = [
    'markers',
    'plot_histograms',
    'plot_images',
    'plot_signals',
    'plot_spectra',
    ]

def __dir__():
    return sorted(__all__)
Copy link
Contributor

@jlaehne jlaehne left a comment

Choose a reason for hiding this comment

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

Great for porting this upstream. Will definetely be a very valuable feature!

doc/user_guide/visualisation.rst Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (224dca7) 80.86% compared to head (39b7a2b) 80.97%.
Report is 10 commits behind head on RELEASE_next_major.

Files Patch % Lines
hyperspy/drawing/_widgets/range.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           RELEASE_next_major    #3224      +/-   ##
======================================================
+ Coverage               80.86%   80.97%   +0.10%     
======================================================
  Files                     140      140              
  Lines                   20399    20458      +59     
  Branches                 4825     4834       +9     
======================================================
+ Hits                    16495    16565      +70     
+ Misses                   2829     2825       -4     
+ Partials                 1075     1068       -7     

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

0Hughman0 and others added 5 commits September 7, 2023 13:36
Fixed grammar.

Co-authored-by: Jonas Lähnemann <jonas@pdi-berlin.de>
Fixed terminology.

Co-authored-by: Jonas Lähnemann <jonas@pdi-berlin.de>
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.

Thank you @0Hughman0, this looks in very good shape!

I think that it would be possible to make more general by supporting the following cases:

  1. signal dimension of 2, use 2D roi instead of 1D
  2. navigation dimension other than 2

Would you interested in trying to implement this in this PR? If not, we (not necessarily including yourself) can try to do this in a follow up PR?

For point 1, I think that we would need to change the name of the function, maybe to something along the line of plot_roi_map in which case, it would be better to do it before the 2.0 release.

@jlaehne
Copy link
Contributor

jlaehne commented Sep 7, 2023

Tests on windows are failing?

@0Hughman0
Copy link
Contributor Author

No problem.

Hyperspy and Lumispy were huge helps over my PhD, happy to contribute.

I can imagine there are other scenarios where similar tools would be useful. As someone who's only really worked with hyperspectral maps, I can find it a bit mindblowing to consider more complex options!

My worry is having an all-in-one implementation for every combination of signals and navigation dimensions could get rather messy and very difficult to implement, document and test.

Is there a way I can get some toy datasets to play with? Don't mind having a go to see how things turn out.

At the risk of going a bit off course in the discussion, I don't know if you have seen or heard of plum - it allows multiple dispatch in Python. Would allow for separating out implementations of method and function calls, depending on the shape of the data. Potentially very useful I think for this library.

@ericpre
Copy link
Member

ericpre commented Sep 8, 2023

I can imagine there are other scenarios where similar tools would be useful. As someone who's only really worked with hyperspectral maps, I can find it a bit mindblowing to consider more complex options!

My worry is having an all-in-one implementation for every combination of signals and navigation dimensions could get rather messy and very difficult to implement, document and test.

It is actually quite simple when using a suitable approach.

Is there a way I can get some toy datasets to play with? Don't mind having a go to see how things turn out.

There are some dataset available at https://pyxem.readthedocs.io/en/stable/open_datasets_workflows.html

At the risk of going a bit off course in the discussion, I don't know if you have seen or heard of plum - it allows multiple dispatch in Python. Would allow for separating out implementations of method and function calls, depending on the shape of the data. Potentially very useful I think for this library.

Thank you for sharing, I can't see straightaway where it would be useful but this is good that it is exist. With the structure of signals classes and the class registration mechanism (https://hyperspy.org/hyperspy-doc/dev/user_guide/signal/signal_basics.html#hyperspy-extensions) we already have a mechanism in place to automatically set the correct class (and functionality) depending on the signal type ("diffraction", "eels", etc.) and dimensionality.

@jlaehne jlaehne added this to the v2.0 Split milestone Sep 8, 2023
@0Hughman0
Copy link
Contributor Author

I'm having a great deal of difficulty getting the pytest-mpl tests to work during CI.

I have made sure I'm using matplotlib==3.5.1, but that doesn't seem to have helped.

Suggestions would be welcome!

Thanks!

@ericpre
Copy link
Member

ericpre commented Sep 13, 2023

Can you check your version of freetype? If needs to be as follow:

- freetype=2.10
- matplotlib-base=3.5.1

I thought that this was documented in developer guide but it is not!

If this doesn't help, we can update the baseline images for you.

@0Hughman0
Copy link
Contributor Author

Thanks, things seem to be passing now.

Failures on windows seem to be something to do with running out of memory when building fonts?

I thought that this was documented in developer guide but it is not!

yes I think maybe the dev install instructions need to be updated for 2.0 perhaps?

I think version 3.5.1 of matplotlib is not compatible with Python 3.11, which I think caused me issues early on.

@0Hughman0
Copy link
Contributor Author

Helloo, happy to put in some time to get this mergable, but not sure what to do next?

@jlaehne
Copy link
Contributor

jlaehne commented Nov 20, 2023

I propose to first get the present functionality merged and leave an implementation for 2D for a separate PR - even if you want to have a go at it, I think it is the approach that is manageable more easily.

As quite a bit has happened to the main branch in the meantime, I would propose to get the current version of RELEASE_next_major merged into this branch and then see what has to be done to get all the tests passing.

hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
doc/user_guide/region_of_interest.rst Outdated Show resolved Hide resolved
upcoming_changes/3224.new.rst Outdated Show resolved Hide resolved
doc/user_guide/region_of_interest.rst Outdated Show resolved Hide resolved
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.

Thank you @0Hughman0, this looks good to me. I did some tweaks and fixed an already existing bug that this PR highlights.

Sorry that it took some times to come back to this PR, but we were busy with other things! :)

@ericpre
Copy link
Member

ericpre commented Nov 30, 2023

@jlaehne, do you want to do a last review?

Copy link
Contributor

@jlaehne jlaehne left a comment

Choose a reason for hiding this comment

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

Just some small improvements to the documentation.

doc/user_guide/region_of_interest.rst Outdated Show resolved Hide resolved
doc/user_guide/region_of_interest.rst Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
upcoming_changes/3224.new.rst Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
hyperspy/drawing/utils.py Outdated Show resolved Hide resolved
@jlaehne jlaehne merged commit b12131d into hyperspy:RELEASE_next_major Dec 1, 2023
23 checks passed
@jlaehne
Copy link
Contributor

jlaehne commented Dec 1, 2023

Hyperspy and Lumispy were huge helps over my PhD, happy to contribute.

Thanks @0Hughman0 - it was a long way, but this has become a really neat feature! If you have other code from your PhD that you want to contribute, we're happy for suggestions :-)

@0Hughman0
Copy link
Contributor Author

This is great! Thanks so much for getting this across the line.

Looking forward to installing 2.0 and having a go!

I'll have a dig through my codebase to see if there's any other handy snippets. If I get the time, I can maybe pick up an issue or two.

My biggest coding project during my PhD was Cassini which is a tool I used to organise my data and analysis. You guys should check it out if you have the time 😁.

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

3 participants