Skip to content

photogrammetry: expose units kwarg, refuse non-mm downstream#175

Merged
emiddell merged 3 commits into
devfrom
photogrammetry-non-mm-inputs
Jul 8, 2026
Merged

photogrammetry: expose units kwarg, refuse non-mm downstream#175
emiddell merged 3 commits into
devfrom
photogrammetry-non-mm-inputs

Conversation

@harmening

@harmening harmening commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Photogrammetry Scans by Scaniverse are by default in meters. While running the face-anonymization pipeline, the landmark picker silently failed because it is currently restricted to 'mm'.

This PR exposes units (and crs) as kwargs to the user, allowing them to provide the correct metadata when loading photogrammetry scans. Converting units still needs to be done by the users themselves, but I added proper NotImplementedErrors to where cedalions photogrammetry pipeline silently assumes 'mm':

  • io/probe_geometry.read_einstar_obj: add crs/units kwargs (default "digitized"/mm) so Scaniverse-style m-scale .obj files can be tagged correctly at load time.
  • io/photogrammetry.{read_photogrammetry_einstar,opt_fid_to_xr}: add units kwarg (default mm); fiducials/optodes now carry units instead of being unit-less.
  • vis/blocks.plot_surface: raise NotImplementedError when pick_landmarks is truthy and the surface isn't in mm — the picker's hard-coded 3-mm sphere radius and 25-mm^2 proximity threshold silently broke on m-scale scans.
  • geometry/photogrammetry/processors.ColoredStickerProcessor: upgrade the mm assert to NotImplementedError (survives -O).
  • geometry/landmarks.LandmarksBuilder1010: refuse non-mm scalp surfaces (previously silently mixed mm-landmarks with a meter-scale mesh).
  • vis/anatomy/optode_selector.OptodeSelector: refuse non-mm surfaces and non-mm points at construction with NotImplementedError. Same class of bug as the plot_surface picker — hard-coded sphere radii (2/3 mm), a 5-mm proximity threshold, and a 6-mm PCA search radius, all silently assumed in mm. Removes two stale # FIXME unit handling comments in plot() now made redundant by the guard.

@harmening harmening requested a review from emiddell July 7, 2026 18:47
@emiddell emiddell merged commit b80eab2 into dev Jul 8, 2026
4 checks passed
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