Update residual line when model line is changed.

[HanHsuanWu]

As suggested in #3352, I rebase to RELEASE_next_patch, as well as cleaning up the commits history.

Currently, the plotted residual is not being updated whenever the model line changes.
So I added update for model._residual_line whenever mode._model_line is being updated.

import numpy as np
import hyperspy.api as hs

# Random Signal1D signal
s = hs.signals.Signal1D(data=np.random.random((2, 2, 1024)), 
                        axes=[{'name': 'x', 'size': 2}, 
                              {'name': 'y', 'size': 2}, 
                              {'name': 'sig', 'size': 1024}])
# Model object with Polynomial component
m = s.create_model()
# Polynomial=hs.model.components1D.Polynomial()
# m.append(Polynomial)
m.append(hs.model.components1D.PowerLaw())
m.plot(plot_residual=True)

[codecov]

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 80.63%. Comparing base (d0f6fe2) to head (e682b2d).
Report is 120 commits behind head on RELEASE_next_patch.

Files	Patch %	Lines
hyperspy/model.py	83.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@                  Coverage Diff                   @@
##           RELEASE_next_patch    #3355      +/-   ##
======================================================
+ Coverage               80.49%   80.63%   +0.14%     
======================================================
  Files                     147      146       -1     
  Lines                   21866    21947      +81     
  Branches                 5149     5176      +27     
======================================================
+ Hits                    17602    17698      +96     
+ Misses                   3045     3030      -15     
  Partials                 1219     1219              

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

[ericpre]

I have suggested some simplication to avoid boilerplate code with the idea that if self._residual_line is not None, then it should be connected.
The same would need to be done for disconnecting the line, when closing the figure and most likely self._residual_line will need to be reset to None - I don't think this is the case at the moment.

I didn't test that my suggestion are corrects (only by reading the code), so I may be missing something!

It would need a test to check that event are connected and disconnected properly, for example something along the lines of:

hyperspy/hyperspy/tests/drawing/test_plot_signal_tools.py

Lines 91 to 100 in 3b350ff

	def test_plot_BackgroundRemoval_close_figure():
	s = signals.Signal1D(np.arange(1000).reshape(10, 100))
	br = BackgroundRemoval(s, background_type="Gaussian")
	signal_plot = s._plot.signal_plot
	assert len(signal_plot.events.closed.connected) == 5
	assert len(s.axes_manager.events.indices_changed.connected) == 4
	s._plot.close()
	assert br._fit not in s.axes_manager.events.indices_changed.connected
	assert br.disconnect not in signal_plot.events.closed.connected

[HanHsuanWu]

nice I will test the changes soon

[HanHsuanWu]

I have suggested some simplication to avoid boilerplate code with the idea that if self._residual_line is not None, then it should be connected. The same would need to be done for disconnecting the line, when closing the figure and most likely self._residual_line will need to be reset to None - I don't think this is the case at the moment.

I didn't test that my suggestion are corrects (only by reading the code), so I may be missing something!

It would need a test to check that event are connected and disconnected properly, for example something along the lines of:

hyperspy/hyperspy/tests/drawing/test_plot_signal_tools.py

Lines 91 to 100 in 3b350ff

	def test_plot_BackgroundRemoval_close_figure():
	s = signals.Signal1D(np.arange(1000).reshape(10, 100))
	br = BackgroundRemoval(s, background_type="Gaussian")
	signal_plot = s._plot.signal_plot
	assert len(signal_plot.events.closed.connected) == 5
	assert len(s.axes_manager.events.indices_changed.connected) == 4
	s._plot.close()
	assert br._fit not in s.axes_manager.events.indices_changed.connected
	assert br.disconnect not in signal_plot.events.closed.connected

I tried running test_plot_BackgroundRemoval_close_figure() and didn't get any error so I think the connection is being closed?
However, I try to use #3316 (comment) ROI to select signal fitting range and this error message comes up after I plot with plot_residual=True.

Traceback (most recent call last):
  File "C:\Users\Han-Hsuan\.conda\envs\hs20\lib\site-packages\matplotlib\cbook\__init__.py", line 287, in process
    func(*args, **kwargs)
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\external\matplotlib\widgets.py", line 210, in onmove
    self._onmove(event)
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\external\matplotlib\widgets.py", line 619, in _onmove
    self.onmove_callback(vmin, vmax)
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\drawing\_widgets\range.py", line 160, in _span_changed
    self.events.changed.trigger(self)
  File "<string>", line 4, in trigger
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\events.py", line 424, in trigger
    function(**{kwf: kwargs[kwt] for kwt, kwf in kwsd.items()})
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\roi.py", line 476, in _on_widget_change
    self.events.changed.trigger(self)
  File "<string>", line 4, in trigger
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\events.py", line 421, in trigger
    function(**{kw: kwargs.get(kw, None) for kw in kwsl})
  File "C:\Users\Han-Hsuan\Documents\GitHub\vibrational_EELS\hs_functions.py", line 498, in _select_signal_range
    hs_model.fit()
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\model.py", line 1682, in fit
    res = leastsq(
  File "C:\Users\Han-Hsuan\.conda\envs\hs20\lib\site-packages\scipy\optimize\_minpack_py.py", line 426, in leastsq
    retval = _minpack._lmdif(func, x0, args, full_output, ftol, xtol,
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\models\model1d.py", line 396, in _errfunc
    errfunc = self._model_function(param) - y
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\model.py", line 1067, in _model_function
    self._fetch_values_from_p0()
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\model.py", line 1041, in _fetch_values_from_p0
    component.fetch_values_from_array(
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\component.py", line 1014, in fetch_values_from_array
    parameter.value = p[i] if length == 1 else p[i : i + length]
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\component.py", line 344, in _set_value
    self.events.value_changed.trigger(value=self.__value, obj=self)
  File "<string>", line 4, in trigger
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\events.py", line 421, in trigger
    function(**{kw: kwargs.get(kw, None) for kw in kwsl})
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\drawing\signal1d.py", line 464, in _auto_update_line
    self.update(self, update_ylimits=update_ylimits, **kwargs)
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\drawing\signal1d.py", line 548, in update
    self.ax.hspy_fig.render_figure()
  File "C:\Users\Han-Hsuan\Documents\GitHub\hyperspy\hyperspy\drawing\figure.py", line 140, in render_figure
    if self.figure.canvas.supports_blit and self._background is not None:
AttributeError: 'NoneType' object has no attribute 'canvas'

[HanHsuanWu]

@ericpre. Took me a while to understand the testing part. But after updating the disconnection, here is a test I made.

def test_plot_residual_close_figure(): 
    s = signals.Signal1D(np.arange(1000).reshape(10, 100)) 
    md = s.create_model()
    md.append(hs.model.components1D.Gaussian())
    md.plot(plot_residual=True)
    signal_plot = s._plot.signal_plot

    assert len(s._plot.signal_plot.events.closed.connected) == 5
    assert len(s.axes_manager.events.indices_changed.connected) == 3
    s._plot.close() 
    print("signal_plot.events.closed.connected:",signal_plot.events.closed.connected)
    print("s.axes_manager.events.indices_changed.connected:",s.axes_manager.events.indices_changed.connected)

test_plot_residual_close_figure()

Here is the result.
signal_plot.events.closed.connected: set()
s.axes_manager.events.indices_changed.connected: {<bound method BaseModel._on_navigating of >}

I am not sure what "<bound method BaseModel._on_navigating of >" is and how to disconnect that.

[ericpre]

@HanHsuanWu, I will look at this in a couple of days.

[ericpre]

This is a function that fetch the current value of the model when the indices are changed and this should stay connected.

It is connected during initialisation of the class:
https://github.com/hyperspy/hyperspy/blob/RELEASE_next_patch/hyperspy/models/model1d.py#L242

and call:

hyperspy/hyperspy/model.py

Lines 997 to 1001 in b742845

	def _on_navigating(self):
	"""Same as fetch_stored_values but without update_on_resume since
	the model plot is updated in the figure update callback.
	"""
	self.fetch_stored_values(only_fixed=False, update_on_resume=False)

If you check that the number of connected functions is the same before plotting and after closing the plot, that should be good enough.