Ticket 1344 slicer #1345
Ticket 1344 slicer #1345
Conversation
And typo in comment.
Default GUI had fold=True forcing -qi to be averaged with +qi which in principle gives better statistics IF symmetric. The general case is not to average. Moreover the average (in sascalc) seems to be forced to fold=False so not clear that in fact we are currently even getting the advantage of statistics!! Also started process of allowing the GUI to use any xmin, xmax; ymin, ymas box, whether or not it includes q=0,0.
|
ready for testing on Win64 |
Fold flag is no longer hard coded for the box slicer. A checkbox is presented to user to toggle between the two states with false as the default. This should also be done for the sector averaging.
Fix breaking of other slicers which don't support a fold at this point and cleanup code. Also add logger to checkbox event code in case a call is made without code to handle it.
|
ready for testing on Win64 |
2 similar comments
|
ready for testing on Win64 |
|
ready for testing on Win64 |
|
Looking at this again I think the rest of what would be nice to have will take much longer and this already fixes a bug and enhances the functionality so asking that it be reviewed and eventually merged. Other enhancements can be added later. |
| math.fabs(self.base.data2D.xmin)) | ||
| if (self.base.data2D.ymax < 0) or (self.base.data2D.ymin > 0): | ||
| self.ymin = self.base.data2D.ymin + 0.25 * self.range |
There was a problem hiding this comment.
self.range is suspicious... presumably you want self.yrange?
| self.yrange = self.base.data2D.ymax - self.base.data2D.ymin | ||
| if (self.base.data2D.xmax < 0) or (self.base.data2D.xmin > 0): | ||
| self.xmin = self.base.data2D.xmin + 0.25 * self.xrange | ||
| self.xmax = self.base.data2D.xmax + 0.25 * self.xrange |
There was a problem hiding this comment.
Is the box supposed to be within the data? In which case you want to set xmax to data xmax minus xrange/4.
There was a problem hiding this comment.
One other note: regardless of this change, the center of the box slicer is always set to Q = 0 with no way to change this in the GUI.
| else: | ||
| self.xmin = -0.5 * min(math.fabs(self.base.data2D.xmax), | ||
| math.fabs(self.base.data2D.xmin)) | ||
| self.xmax = 0.5 * min(math.fabs(self.base.data2D.xmax), |
There was a problem hiding this comment.
This is just
self.xmin = -self.x
self.xmax = self.x
| math.fabs(self.base.data2D.xmin)) | ||
| if (self.base.data2D.ymax < 0) or (self.base.data2D.ymin > 0): | ||
| self.ymin = self.base.data2D.ymin + 0.25 * self.range | ||
| self.ymax = self.base.data2D.ymax + 0.25 * self.yrange |
There was a problem hiding this comment.
Again, plus a quarter or minus a quarter?
| @@ -253,6 +275,7 @@ def get_params(self): | |||
| params["x_max"] = math.fabs(self.vertical_lines.x) | |||
| params["y_max"] = math.fabs(self.horizontal_lines.y) | |||
| params["nbins"] = self.nbins | |||
| params["abs q"] = self.fold | |||
There was a problem hiding this comment.
x_max and y_max don't have spaces. Do you want to use abs_q instead?
| @@ -199,7 +211,7 @@ def set_slicer(self, type, params): | |||
| iy += 1 | |||
| self.auto_save = wx.CheckBox(parent=self, id=wx.NewId(), | |||
| label="Auto save generated 1D:") | |||
| self.Bind(wx.EVT_CHECKBOX, self.on_auto_save_checked) | |||
| self.Bind(wx.EVT_CHECKBOX, self.on_checbox_checked) | |||
| _cb_not_found = True | ||
| _param_index = next((i for i, v in enumerate(self.parameters) | ||
| if v[0] == "abs q"), None) | ||
| if (_param_index != None): |
There was a problem hiding this comment.
To compare with None use
if _param_index is not None:
| execute. For this class the two checkbox events are used to: | ||
|
|
||
| a) Enable/Disable auto append | ||
| b) togle between +/-q or abs q (|q|) slicer averaging/plotting |
| if (_param_index != None): | ||
| if _cb == self.abs_box: | ||
| _cb_not_found = False | ||
| (self.parameters[_param_index])[1] = _cb |
There was a problem hiding this comment.
It is odd that checking the checkbox for abs q will set the control associated in abs q to that checkbox. Shouldn't the proper control have been put into the parameter list when the control was created?
| if _cb_not_found: | ||
| self.msg = "Checkbox generating event does not seem to be active." | ||
| logger.info(self.msg) | ||
|
|
There was a problem hiding this comment.
[Note: simplified for loop---don't need the index since we are updating the item in place]
Maybe use:
cb = evt.GetEventObject()
# check for abs q folding
if cb == self.abs_box:
for item in self.parameters:
if item[0] == "abs q":
item[1] = cb
self.on_text_enter(wx.EVT_CHECKBOX)
return
# fall through to error logging if abs q not in parameter list
# check for auto save
if cb == self.auto_save:
checked = self.auto_save.IsChecked()
self.append_name.Enable(checked)
self.path.Enable(checked)
self.fitting_options.Enable(checked)
return
# Fall through for other kinds of boxes
logger.info("Checkbox generating event does not seem to be active.")
|
ready for testing on Win64 |
|
Closing PR but keeping the branch |
This branch aims to allow the GUI to provide arbitrary box slicing (defined by xmax, xmin, ymax, ymin) and for boxes crossing Q=0,0 the option to "fold" or not.
NOTE: this is NOT ready for checking, but the first feature (allowing for + and - q to be plotted separately is needed by a user right now so providing this build as a stopgap till the fixes are finished.
NOTE2: This is written on the 4x series but the code (including the mistakes:-) seems to have been copied and pasted to the 5.0 so once we figure it out the changes should port easily to 5.x.