Automatic feature selection, and one point-selection window - #67
Open
klemengit wants to merge 18 commits into
Open
Automatic feature selection, and one point-selection window#67klemengit wants to merge 18 commits into
klemengit wants to merge 18 commits into
Conversation
pyidi had five separate point-selection implementations. Three were dead code, one (SubsetSelection) was documented but no longer developed, and the one under active development (SelectionGUI) was not reachable from the documented workflow. Retire SubsetSelection and delete pyidi/GUIs/selection.py. The name stays importable but raises RuntimeError naming the replacement, so existing scripts fail with an actionable message rather than an ImportError. Move the ROI-grid geometry into pyidi/selection_geometry.py, a pure-numpy module with no GUI-toolkit dependency, shared by the napari GUI and SelectionGUI. get_roi_grid is kept rather than dropped: it is the only one supporting an anisotropic roi_size and a deselect polygon, both of which gui.py needs. The functions do not share one coordinate convention; each docstring states which it uses and the tests pin the difference. Harden set_points(). Empty, non-2-D, wrong-column-count and out-of-bounds input now raise ValueError instead of being accepted silently or failing with IndexError. Sub-pixel points are rounded to nearest with a warning, replacing a split where the same float input crashed SimplifiedOpticalFlow but was silently truncated toward zero in the other three methods. It also duck-types on a .points attribute, so a selection GUI can be passed directly. The napari GUI now routes its selections through it too. Fix SelectionGUI accepting a numpy array, as its docstring claimed; a 2-D or 3-D array previously raised AttributeError. Remove the dead selection code: tools.ManualROI, tools.GridOfROI (both read a video.reader.mraw attribute that no longer exists), the unreachable PickPoints class, and the stray "load_analysis copy.py". Add the first tests for the GUI package (24), point the docs at SelectionGUI, and fix a README line telling users to call video.set_points(), which VideoReader has never had.
Deleting the last remaining grid or polyline silently did nothing: both delete handlers guarded on len(...) > 1, so the delete was skipped and neither a status message nor a disabled button said why. The guard existed because the click handlers index the list directly and an empty list would raise IndexError on the next click. Delete now works and re-seeds an empty entry, preserving that invariant properly. Polygon and grid vertices can be dragged. A left-drag starting within ~10 screen pixels of a vertex moves it; a drag anywhere else still pans. The grab radius is derived from viewPixelSize so it stays constant in screen space at any zoom. Hit-testing covers every grid and polyline, not just the active one, and works on the vertex lists rather than the derived subset points, which are a different structure in the opposite coordinate order. Clicking exactly on a vertex is now a no-op instead of dropping a duplicate on top of it. recompute_roi_points re-derives every grid, polyline and brush mask, and update_selected_points rebuilds a full-frame RGBA overlay, so neither can run per mouse-move. The drag updates only the outline during the move and recomputes once on release. Add Ctrl+Z undo for adding a vertex, moving a vertex, and deleting a grid or polyline, bounded at 50 entries. A restored grid returns to its original row with its original label. Manual points, brush strokes and filter results are not undoable. Fix a 9 px offset between the cursor and every mouse drag. The drag handlers read ev.pos()/ev.buttonDownPos(), which are local to the ViewBox, and passed them to mapSceneToView() and sceneBoundingRect().contains(), which expect scene coordinates; the ViewBox sits at scene (9, 9). The click handlers already used scenePos(), so clicking and dragging disagreed by more than the 10 px grab radius, which would have made vertex grabbing miss every time. Two of the three sites predate vertex dragging: the brush has always painted about 9 px from the cursor, and its bounds check was wrong by the same amount. Label the shared button "Start new grid" in Grid mode, and correct the status hint that read "Click 'Start new line' to begin a new grid". Extracting the drag branches drops BrushViewBox.mouseDragEvent from complexity 13 to a short dispatcher, clearing the file's only C901.
The pyIDI class in pyidi/pyidi.py has raised NotImplementedError since 1.0, pointing callers at pyidi_legacy.pyIDI, which is what __init__.py actually exports. Nothing imported the module: the only import of it was already commented out. Every apparent reference elsewhere in the repo and docs is to pyidi.pyIDI, the legacy class, which is unaffected.
Several commits landed after the 1.4.0 section was written and were missing from the changelog entirely: Eulerian video magnification, rigid body motion in DirectionalLucasKanade, the NaN-contract and asymmetric-pad kernel fixes, and the restored 1.3.3 Lucas-Kanade helper aliases.
SubsetSelection accepted an anisotropic roi_size=(y, x); SelectionGUI replaced it with a scalar subset_size and could only select square subsets, so LucasKanade users could not pick a non-square ROI in the UI. subset_size now accepts a scalar or a (height, width) pair, in the same (vertical, horizontal) convention as LucasKanade.configure(roi_size=...), and a Square subsets checkbox in the panel keeps the previous behaviour by default. The geometry helpers take the pair per axis, so the grid step is height + overlap down and width + overlap across. Along the line is the subtle case: the step is the extent of the subset projected on the segment direction, sqrt((dx*w)**2 + (dy*h)**2), chosen because it collapses to exactly the old scalar step at every angle for a square subset. The obvious alternative, |dx|*w + |dy|*h, would have silently changed the spacing of existing diagonal polylines by a factor of 1.4. The image and overlay arrays are column-major, so index 0 is x/width and index 1 is y/height, while selected_points holds (x, y) and was being unpacked as 'for y, x in ...'. The two inversions cancelled, which made the naming harmless for square subsets and wrong the moment the axes differed: the subset rectangles and both filter ROIs came out transposed. The locals in those three functions are renamed to say what they hold, and the tests pin the extents.
selection.gif showed the tkinter selection UI that was removed in the SelectionGUI consolidation, and the docs page carried a note apologising for it. The replacement is generated from the current PyQt6 GUI by a script kept next to it, so it can be regenerated when the panel changes again. Rendering is headless: SelectionGUI ends its constructor in sys.exit(app.exec()) when sys.ps1 is absent, so the script sets sys.ps1 and stubs QApplication.exec. The asset drops from 1.6 MB to 58 kB.
Restructure the docs around what has shipped since the last release and document the parts that had no documentation at all. New pages: - Eulerian video magnification: the feature was entirely undocumented. Covers what it is for, choosing a band and a gain, the frame count the temporal filter needs, the region-of-interest mask, lambda_c, and save(), with the visualization-not-a-measurement warning stated throughout. - Reading a video: every supported format including the new .cine, colour and bit-depth handling, and the frame-rate caveat that containers often report a playback rate rather than the capture rate. - Results, saving and reloading: the analysis directory layout, load_analysis, resuming from a checkpoint, and what a NaN in the result means. - Upgrading: SubsetSelection -> SelectionGUI (including the noverlap -> subset_overlap sign change), use_numba -> use_compiled_kernel, the stricter set_points() contract, NaN instead of aborting, and the pre-1.0 pyIDI class. - Changelog, rendered from CHANGELOG.md. Rewritten: the landing page (card grid, a runnable quick start, four grouped toctrees); the methods page, which gains a method-comparison table, a parameter table per method and a section on prescribed rigid-body motion in DirectionalLucasKanade; the tutorial; installation; the API reference; and the contributing page. The mode-shape magnification and fiducial-marker pages were stubs reading "more documentation is coming soon" and now document the actual API. Sphinx gains sphinx-design (the landing-page cards), myst-parser (Markdown), napoleon (fiducial.py uses Google-style docstrings, which were rendering as plain text) and intersphinx. The build is warning-free. Two source docstrings are corrected along the way: ResultViewer documented its displacements argument as (n_frames, n_points, 2) while indexing it as (n_points, n_frames, 2), which is what get_displacements returns; and VideoReader.get_frame had a mis-indented field that broke its rendering.
The four parallel stores -- manual points, polylines, grids and brush strokes -- become one ordered list of entries, replacing the two mode-specific lists that were only visible while their own tool was active. Four fixes fall out of it: removed points no longer reappear, a deselect stroke is subtracted from a brush mask instead of discarding the whole stroke, filter candidates are re-derived on every selection change, and the brush had its row and column steps swapped for a non-square subset_size. Subset borders move into a cosmetic-pen path so they stay a hairline at any zoom, and the interior is built with whole-array numpy: 3x faster at 5000 subsets. get_points() now returns creation order rather than grouped by type.
…d its GUI Selection could only find features it already happened to sample, and the spacing could not be lowered because scoring per subset took minutes at a megapixel. Scoring the whole image at once removes that constraint: Shi-Tomasi as a Sobel plus three box sums and a closed-form eigenvalue is the same quantity three orders of magnitude faster. pyidi/selection/ splits the steps issue ladisk#51 converged on -- regions define an area, the image is scored and cached whole, and a threshold plus greedy minimum-distance suppression turns the score into features. FeatureSelectionGUI is the interface over it. Only evaluation is expensive, so masking and every slider re-derive from the cached score; the tests assert that by counting evaluator runs. No Qt in the package, SelectionGUI untouched, class name provisional.
Find first, then trim: two tabs named after the steps, seeded with a 'Whole image' row, rather than three numbered ones starting on an empty frame. Quality replaces the percentile threshold. A dense score image is overwhelmingly background -- its 90th percentile was 0.0016 of the best feature -- so most of the slider sat inside the featureless area and lowering it admitted background instead of relaxing the bar. The separation replaces the minimum distance and the point cap as the density control. Distance 0 gave tighter clusters than distance 2, which was the cap keeping the highest scorers, all on the same few features. Decimation is no substitute: thinned to twenty thousand points, every n-th leaves 78% of pairs within three pixels where the separation leaves none. Three tiers of point on the mask tab, so an empty patch tells "nothing there" apart from "you masked it away". Odd subset sizes only. Flatter panel, which is what was clipping its values. Draggable gradient direction. A threshold drag was 80 ms a step, mostly Python loops over the points rather than the selection. Vectorised, rasters cached, one pipeline pass per redraw, candidates reduced before the walk, redraws coalesced: 26 ms.
A brush move cost 24 ms and a polygon corner 115 ms at seventeen thousand points, none of it the selection algorithm. Painting rebuilt a whole-frame overlay per dab and redrew the whole point cloud to cross out the covered points; it now draws a path of discs and puts the crosses over the red points. A masked selection ran over the whole frame and now runs in the mask's bounding box, snapped to a whole reduction cell so the points are identical. The per-point occupancy stamp is skipped unless a later mask group reads it. The two uniform point layers are one stroked path each rather than ScatterPlotItems -- Qt draws nothing for a degenerate subpath, so a test renders the canvas to prove the dots are on it. Brush move: 24 -> 0.1 ms. Corner: 115 -> 30 ms. Redraw: 342 -> 158 ms.
The four conflicts are all additive. `datasets.rst` and `quick_start/video_reader.rst` both stay in the getting-started toctree, and the API reference gains the datasets module alongside the methods. The README section is rewritten in the current voice — PR ladisk#66 predates the README overhaul in bfee5d7 and still used the old ALL-CAPS headings. In the changelog, `Example datasets` joins the unreleased sections and the `show_pbar` fix becomes a bullet in the existing `Fixed` list.
The 1.3 window becomes SelectionGUIOld. `feature_selection.rst` takes over the `point-selection` anchor, so cross-references written against the old name land on the interface that name now means.
PyQt6 present with napari absent made `import pyidi` fail outright. Availability is checked with `find_spec`, so a genuine ImportError inside a module still propagates instead of being swallowed into a stub.
`Brush radius` follows the tools that paint; `Point spacing` follows a row that lays points out along or inside its shape, which is the only thing that reads it.
`deselect_mode` becomes a property reading the active tool, so there is no longer a stored flag that can disagree with it.
…theme Only `:checked` is styled, so the window does not reintroduce the whole custom theme SelectionGUIOld carries.
Contributor
|
looks great to me! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The unreleased work since 1.3.3.
Automatic feature selection. A new
pyidi.selectionpackage — mask,evaluate, select — scores the whole frame once and picks the best-separated
features inside the region you drew, instead of placing a grid and then
discarding the poor ones. On a speckle pattern that is the difference between
sampling where the features are and where the grid happens to fall. It imports
without Qt, so the pipeline is scriptable and the GUI is just a front end.
Implements the workflow discussed in #51, using the vocabulary agreed there.
One point-selection window.
SelectionGUInow names that interface; the1.3 window becomes
SelectionGUIOld, deprecated and removed in 1.5. The newone turned out to be a superset rather than a companion — every selection
method has a counterpart tool, both filters are evaluators — so keeping both
meant two ways to do the same five things. The constructor signature and the
(row, col)return are unchanged, so most scripts need no edit; the upgradingguide covers what does not carry over.
Also: anisotropic subsets, vertex editing and undo, a documentation
overhaul, and a fix for
import pyidifailing outright when PyQt6 wasinstalled without napari.
Full account in
CHANGELOG.mdunder Unreleased. 440 tests pass.