Skip to content

fix hx/hy assignment regression for line profiles#1660

Open
barentine wants to merge 2 commits into
python-microscopy:masterfrom
barentine:linefix260422
Open

fix hx/hy assignment regression for line profiles#1660
barentine wants to merge 2 commits into
python-microscopy:masterfrom
barentine:linefix260422

Conversation

@barentine
Copy link
Copy Markdown
Member

Addresses regression in arrayViewPanel (commit here) where hx, hy never get assigned for line profiles (width=1) .

Is this a bugfix or an enhancement?
bugfix
Proposed changes:

  • call do.GetSliceSelection() for line profiles (width=1)
  • round to get pixel identity rather than casting to int, which is a floor operation.
image produces
>>> lx, ly, hx, hy = do.GetSliceSelection()
>>> do.GetSliceSelection()
(0, 0, 0, 6)

while
image

yields

>>> (xi, yi, xf, yf)
(0, 0, 1, 1)
>>> image.data_xytc[xi:xf+1, yi:yf+1,0,0]
array([[ 0.25463662,  2.1478719 ],
       [ 1.99672588, -1.52296621]])

There are some remaining quirks:

  • displayOptions.GetSliceSelection returning start, finish, and us calling things lx, hx is fairly misleading and requires a +1 for the finish to get the full extent.
  • There appears to be a bug in PYME.Analysis.profile_extraction import extract_profile when drawing along the edge of the frame.

tested on
wxPython '4.2.5 msw (phoenix) wxWidgets 3.2.9'
python 3.11
Windows 11

@David-Baddeley
Copy link
Copy Markdown
Contributor

I need to understand the implications of round vs floor a bit better. Naively I'd think the correct behaviour for square selections at least is probably floor on the top left and ceil on the bottom right. For lines we want the same behaviour for top left and bottom right (round, floor, or ceil) (but I'd like to avoid selection type logic if I can in anything but display).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants