Skip to content

Commit

Permalink
Remove use of QSignalBlocker in most code
Browse files Browse the repository at this point in the history
It appears that we can't always rely on python's reference counting
system to delete `QSignalBlocker` (or lists of `QSignalBlocker`) at
the end of method calls. As such, we should convert over to using the
`block_signals()` context manager from `hexrd.ui.utils`.

This commit moves all code over to that.

Signed-off-by: Patrick Avery <[email protected]>
  • Loading branch information
psavery committed Jul 19, 2022
1 parent 332abe9 commit 7e90c01
Show file tree
Hide file tree
Showing 17 changed files with 359 additions and 361 deletions.
20 changes: 10 additions & 10 deletions hexrd/ui/calibration_crystal_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import numpy as np
from numpy.linalg import LinAlgError

from PySide2.QtCore import QObject, QSignalBlocker, Signal
from PySide2.QtCore import QObject, Signal
from PySide2.QtWidgets import QFileDialog

from hexrd import instrument, matrixutil
Expand All @@ -16,7 +16,7 @@
from hexrd.ui.select_grains_dialog import SelectGrainsDialog
from hexrd.ui.select_items_widget import SelectItemsWidget
from hexrd.ui.ui_loader import UiLoader
from hexrd.ui.utils import convert_angle_convention
from hexrd.ui.utils import block_signals, convert_angle_convention


class CalibrationCrystalEditor(QObject):
Expand Down Expand Up @@ -171,8 +171,8 @@ def update_duplicate(self, w):
dup_ind = self.stretch_matrix_duplicates.get(ind)
if dup_ind is not None:
dup = getattr(self.ui, f'stretch_matrix_{dup_ind}')
blocker = QSignalBlocker(dup) # noqa: F841
dup.setValue(w.value())
with block_signals(dup):
dup.setValue(w.value())

def set_matrix_valid(self):
self.set_matrix_style_sheet('background-color: white')
Expand Down Expand Up @@ -215,8 +215,8 @@ def orientation(self, v):
v = np.degrees(v)

for i, w in enumerate(self.orientation_widgets):
blocker = QSignalBlocker(w) # noqa: F841
w.setValue(v[i])
with block_signals(w):
w.setValue(v[i])

@property
def position(self):
Expand All @@ -225,8 +225,8 @@ def position(self):
@position.setter
def position(self, v):
for i, w in enumerate(self.position_widgets):
blocker = QSignalBlocker(w) # noqa: F841
w.setValue(v[i])
with block_signals(w):
w.setValue(v[i])

@property
def inverse_stretch(self):
Expand All @@ -245,8 +245,8 @@ def stretch_matrix(self):
@stretch_matrix.setter
def stretch_matrix(self, v):
for i, w in enumerate(self.stretch_matrix_widgets):
blocker = QSignalBlocker(w) # noqa: F841
w.setValue(v[i])
with block_signals(w):
w.setValue(v[i])

@property
def orientation_widgets(self):
Expand Down
26 changes: 13 additions & 13 deletions hexrd/ui/calibration_crystal_slider_widget.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from enum import IntEnum

from PySide2.QtCore import QObject, QSignalBlocker, Signal
from PySide2.QtCore import QObject, Signal
from PySide2.QtWidgets import QProxyStyle, QStyle

from hexrd.ui.ui_loader import UiLoader
from hexrd.ui.utils import block_signals


class SpinBoxStyle(QProxyStyle):
Expand Down Expand Up @@ -90,9 +91,9 @@ def on_mode_changed(self):

# Update spinbox values
for i, w in enumerate(self.spinbox_widgets):
blocker = QSignalBlocker(w) # noqa: F841
w.setSuffix(suffix)
w.setValue(data[i])
with block_signals(w):
w.setSuffix(suffix)
w.setValue(data[i])

# Update slider positions
self.ui.slider_range.setValue(srange)
Expand Down Expand Up @@ -123,10 +124,9 @@ def on_spinbox_changed(self, value):
slider_value = value * self.CONF_VAL_TO_SLIDER_VAL
w_name = f'slider_{index}'
w = getattr(self.ui, w_name)
blocker = QSignalBlocker(w) # noqa: F841
w.setValue(slider_value)

self.changed.emit(mode.value, index, value)
with block_signals(w):
w.setValue(slider_value)
self.changed.emit(mode.value, index, value)

def reset_ranges(self):
self._orientation_range = self.DEFAULT_SLIDER_RANGE
Expand Down Expand Up @@ -156,8 +156,8 @@ def update_gui(self, orientation, position):
data = self._orientation if self.mode == WidgetMode.ORIENTATION \
else self._position
for i, w in enumerate(self.spinbox_widgets):
blocker = QSignalBlocker(w) # noqa: F841
w.setValue(data[i])
with block_signals(w):
w.setValue(data[i])
self.update_ranges()

def update_ranges(self):
Expand All @@ -169,6 +169,6 @@ def update_ranges(self):
sliders = self.slider_widgets
for i, slider in enumerate(sliders):
val = data[i] * self.CONF_VAL_TO_SLIDER_VAL
blocker = QSignalBlocker(slider) # noqa: F841
slider.setRange(val - delta, val + delta)
slider.setValue(val)
with block_signals(slider):
slider.setRange(val - delta, val + delta)
slider.setValue(val)
16 changes: 9 additions & 7 deletions hexrd/ui/dev-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ class SomeWidget(QObject):
For updating the GUI with internal config, the design pattern we typically
use is as follows:
```
from hexrd.ui.utils import block_signals
...
@property
def all_widgets(self):
return [
Expand All @@ -118,16 +122,14 @@ use is as follows:
]
def update_gui(self):
blockers = [QSignalBlocker(x) for x in self.all_widgets] # noqa: F841
self.ui.widget1.setValue(...)
...
with block_signals(*self.all_widgets):
self.ui.widget1.setValue(...)
...
```

We need to block the widget signals when we are updating the values, so that
they do not modify the config as well. The reason we use a list of
QSignalBlockers is so that if an exception is raised, they will be unblocked
automatically. `# noqa: F841` is necessary to tell `flake8` to ignore the
fact that we don't use `blockers` (it is only being used in an RAII fashion).
they do not modify the config as well. The reason we use a context manager
is so that if an exception is raised, they will be unblocked automatically.

Resources
---------
Expand Down
93 changes: 47 additions & 46 deletions hexrd/ui/image_mode_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
import multiprocessing
import numpy as np

from PySide2.QtCore import QObject, QSignalBlocker, Signal
from PySide2.QtCore import QObject, Signal

from hexrd.ui.constants import ViewType
from hexrd.ui.create_hedm_instrument import create_hedm_instrument
from hexrd.ui.create_raw_mask import apply_threshold_mask, remove_threshold_mask
from hexrd.ui.hexrd_config import HexrdConfig
from hexrd.ui.ui_loader import UiLoader
from hexrd.ui.utils import block_signals


class ImageModeWidget(QObject):
Expand Down Expand Up @@ -133,45 +134,45 @@ def all_widgets(self):
return widgets

def update_gui_from_config(self):
blocked = [QSignalBlocker(x) for x in self.all_widgets()] # noqa: F841
self.ui.raw_threshold_comparison.setCurrentIndex(
HexrdConfig().threshold_comparison)
self.ui.raw_threshold_value.setValue(
HexrdConfig().threshold_value)
self.ui.raw_threshold_mask.setChecked(
HexrdConfig().threshold_mask_status)
self.ui.cartesian_pixel_size.setValue(
HexrdConfig().cartesian_pixel_size)
self.ui.cartesian_virtual_plane_distance.setValue(
HexrdConfig().cartesian_virtual_plane_distance)
self.ui.cartesian_plane_normal_rotate_x.setValue(
HexrdConfig().cartesian_plane_normal_rotate_x)
self.ui.cartesian_plane_normal_rotate_y.setValue(
HexrdConfig().cartesian_plane_normal_rotate_y)
self.ui.polar_pixel_size_tth.setValue(
HexrdConfig().polar_pixel_size_tth)
self.ui.polar_pixel_size_eta.setValue(
HexrdConfig().polar_pixel_size_eta)
self.ui.polar_res_tth_min.setValue(
HexrdConfig().polar_res_tth_min)
self.ui.polar_res_tth_max.setValue(
HexrdConfig().polar_res_tth_max)
self.ui.polar_res_eta_min.setValue(
HexrdConfig().polar_res_eta_min)
self.ui.polar_res_eta_max.setValue(
HexrdConfig().polar_res_eta_max)
self.ui.polar_snip1d_algorithm.setCurrentIndex(
HexrdConfig().polar_snip1d_algorithm)
self.ui.polar_apply_snip1d.setChecked(
HexrdConfig().polar_apply_snip1d)
self.ui.polar_snip1d_width.setValue(
HexrdConfig().polar_snip1d_width)
self.ui.polar_snip1d_numiter.setValue(
HexrdConfig().polar_snip1d_numiter)
self.ui.polar_apply_erosion.setChecked(
HexrdConfig().polar_apply_erosion)

self.update_enable_states()
with block_signals(*self.all_widgets()):
self.ui.raw_threshold_comparison.setCurrentIndex(
HexrdConfig().threshold_comparison)
self.ui.raw_threshold_value.setValue(
HexrdConfig().threshold_value)
self.ui.raw_threshold_mask.setChecked(
HexrdConfig().threshold_mask_status)
self.ui.cartesian_pixel_size.setValue(
HexrdConfig().cartesian_pixel_size)
self.ui.cartesian_virtual_plane_distance.setValue(
HexrdConfig().cartesian_virtual_plane_distance)
self.ui.cartesian_plane_normal_rotate_x.setValue(
HexrdConfig().cartesian_plane_normal_rotate_x)
self.ui.cartesian_plane_normal_rotate_y.setValue(
HexrdConfig().cartesian_plane_normal_rotate_y)
self.ui.polar_pixel_size_tth.setValue(
HexrdConfig().polar_pixel_size_tth)
self.ui.polar_pixel_size_eta.setValue(
HexrdConfig().polar_pixel_size_eta)
self.ui.polar_res_tth_min.setValue(
HexrdConfig().polar_res_tth_min)
self.ui.polar_res_tth_max.setValue(
HexrdConfig().polar_res_tth_max)
self.ui.polar_res_eta_min.setValue(
HexrdConfig().polar_res_eta_min)
self.ui.polar_res_eta_max.setValue(
HexrdConfig().polar_res_eta_max)
self.ui.polar_snip1d_algorithm.setCurrentIndex(
HexrdConfig().polar_snip1d_algorithm)
self.ui.polar_apply_snip1d.setChecked(
HexrdConfig().polar_apply_snip1d)
self.ui.polar_snip1d_width.setValue(
HexrdConfig().polar_snip1d_width)
self.ui.polar_snip1d_numiter.setValue(
HexrdConfig().polar_snip1d_numiter)
self.ui.polar_apply_erosion.setChecked(
HexrdConfig().polar_apply_erosion)

self.update_enable_states()

def update_enable_states(self):
apply_snip1d = self.ui.polar_apply_snip1d.isChecked()
Expand Down Expand Up @@ -280,9 +281,9 @@ def on_eta_min_changed(self, min_value):
max_value = min_value + 360.0
update_max = True
if update_max:
blocked = QSignalBlocker(self.ui.polar_res_eta_max) # noqa: F841
self.ui.polar_res_eta_max.setValue(max_value)
HexrdConfig().set_polar_res_eta_max(max_value, rerender=False)
with block_signals(self.ui.polar_res_eta_max):
self.ui.polar_res_eta_max.setValue(max_value)
HexrdConfig().set_polar_res_eta_max(max_value, rerender=False)
HexrdConfig().polar_res_eta_min = min_value

def on_eta_max_changed(self, max_value):
Expand All @@ -296,9 +297,9 @@ def on_eta_max_changed(self, max_value):
min_value = max_value - 360.0
update_min = True
if update_min:
blocked = QSignalBlocker(self.ui.polar_res_eta_min) # noqa: F841
self.ui.polar_res_eta_min.setValue(min_value)
HexrdConfig().set_polar_res_eta_min(min_value, rerender=False)
with block_signals(self.ui.polar_res_eta_min):
self.ui.polar_res_eta_min.setValue(min_value)
HexrdConfig().set_polar_res_eta_min(min_value, rerender=False)
HexrdConfig().polar_res_eta_max = max_value


Expand Down
85 changes: 43 additions & 42 deletions hexrd/ui/indexing/fit_grains_options_dialog.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
from PySide2.QtCore import (
QItemSelectionModel, QModelIndex, QObject, QSignalBlocker, Qt, Signal)
QItemSelectionModel, QModelIndex, QObject, Qt, Signal)
from PySide2.QtWidgets import QDialogButtonBox, QFileDialog, QHeaderView

from hexrd.ui.hexrd_config import HexrdConfig
from hexrd.ui.indexing.grains_table_model import GrainsTableModel
from hexrd.ui.reflections_table import ReflectionsTable
from hexrd.ui.ui_loader import UiLoader
from hexrd.ui.utils import block_signals

from hexrd.ui.indexing.fit_grains_tolerances_model import (
FitGrainsToleranceModel)
Expand Down Expand Up @@ -221,47 +222,47 @@ def update_config(self):
indexing_config['_write_spots'] = self.ui.write_out_spots.isChecked()

def update_gui_from_config(self, config):
blocked = [QSignalBlocker(x) for x in self.all_widgets()]
self.ui.npdiv.setValue(config.get('npdiv'))
self.ui.refit_pixel_scale.setValue(config.get('refit')[0])
self.ui.refit_ome_step_scale.setValue(config.get('refit')[1])
self.ui.threshold.setValue(config.get('threshold'))

tth_max = config.get('tth_max')
if isinstance(tth_max, bool):
enabled = tth_max
instrument = tth_max
value = 0.0
else:
enabled = True
instrument = False
value = tth_max

self.ui.tth_max_enable.setChecked(enabled)

self.ui.tth_max_instrument.setEnabled(enabled)
self.ui.tth_max_instrument.setChecked(instrument)

self.ui.tth_max_specify.setEnabled(enabled)
self.ui.tth_max_specify.setChecked(not instrument)

self.ui.tth_max_value.setEnabled(enabled and (not instrument))
self.ui.tth_max_value.setValue(value)

tolerances = config.get('tolerance')
self.tolerances_model.update_from_config(tolerances)

indexing_config = HexrdConfig().indexing_config
self.selected_material = indexing_config.get('_selected_material')
working_dir = indexing_config.get(
'working_dir', str(Path(HexrdConfig().working_dir).parent))
analysis_name = indexing_config.get(
'analysis_name', Path(HexrdConfig().working_dir).stem)
self.spots_path = str(Path(working_dir) / analysis_name)
write_spots = indexing_config.get('_write_spots', False)
self.ui.write_out_spots.setChecked(write_spots)

self.update_num_hkls()
with block_signals(*self.all_widgets()):
self.ui.npdiv.setValue(config.get('npdiv'))
self.ui.refit_pixel_scale.setValue(config.get('refit')[0])
self.ui.refit_ome_step_scale.setValue(config.get('refit')[1])
self.ui.threshold.setValue(config.get('threshold'))

tth_max = config.get('tth_max')
if isinstance(tth_max, bool):
enabled = tth_max
instrument = tth_max
value = 0.0
else:
enabled = True
instrument = False
value = tth_max

self.ui.tth_max_enable.setChecked(enabled)

self.ui.tth_max_instrument.setEnabled(enabled)
self.ui.tth_max_instrument.setChecked(instrument)

self.ui.tth_max_specify.setEnabled(enabled)
self.ui.tth_max_specify.setChecked(not instrument)

self.ui.tth_max_value.setEnabled(enabled and (not instrument))
self.ui.tth_max_value.setValue(value)

tolerances = config.get('tolerance')
self.tolerances_model.update_from_config(tolerances)

indexing_config = HexrdConfig().indexing_config
self.selected_material = indexing_config.get('_selected_material')
working_dir = indexing_config.get(
'working_dir', str(Path(HexrdConfig().working_dir).parent))
analysis_name = indexing_config.get(
'analysis_name', Path(HexrdConfig().working_dir).stem)
self.spots_path = str(Path(working_dir) / analysis_name)
write_spots = indexing_config.get('_write_spots', False)
self.ui.write_out_spots.setChecked(write_spots)

self.update_num_hkls()

def run(self):
self.ui.show()
Expand Down
Loading

0 comments on commit 7e90c01

Please sign in to comment.