ROI: Provide feature for snapSize (x,y) #2485
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| ## snap | ||
| if self.scaleSnap or (modifiers & QtCore.Qt.KeyboardModifier.ControlModifier): | ||
| lp1[0] = round(lp1[0] / self.scaleSnapSize) * self.scaleSnapSize | ||
| lp1[1] = round(lp1[1] / self.scaleSnapSize) * self.scaleSnapSize | ||
| lp1[0] = np.floor(lp1[0] / self._scaleSnapSize[0]) * self._scaleSnapSize[0] |
Check failure
Code scanning / CodeQL
Potentially uninitialized local variable
|
CI says no due to codescan issue from before, but as the diff involves a line it already had an issue with, it's returning an error. I'll let @outofculture decide on merging. Thanks for the PR @dgoeries ! |
|
Maybe this was discussed elsewhere already, but could we avoid the deprecation by accepting either a single scalar or a sequence? It's a minor convenience if you don't care about independent x/y, but no breaking change. I suppose setting |
|
@ixjlyons Yes, we can also provide both ways for an amount of time. Looking forward for @outofculture opinion ... |
|
So sorry for the long delay on this. This feature is a great addition. Yes, absolutely, this will need to have a deprecation cycle if we don't also build in the ability to use the old interface. I think it's reasonable to do both, though. All the old tests (thanks again for those) will need to be copied to ensure that old behavior continues, too. Are you able to put this change together? If not, I can try to take it on. |
This is the next PR after #2476
We now allow to declare the snapSize of the ROI as sequence of length 2 to have individual snapping for x and y dimension.
I extended the previously implemented tests. By doing so, I realized that flooring will provide a better snapping than rounding.