Structured angular sizes (SizeObject) with catalog image overlays#393
Merged
Conversation
Replace plain string size field with SizeObject class that stores angular extents in arcseconds, mirroring the MagnitudeObject pattern. - SizeObject stores extents internally in arcseconds with constructors from_arcmin, from_arcsec, from_degrees for type-safe unit handling - JSON serialization (to_json/from_json) for DB storage - Auto-selects display unit (°, ', ") based on magnitude - filter_size field (max extent in arcminutes) enables future filtering - Variable-length extents list: 1=circle, 2=ellipse, N=polygon - Update all 12 catalog loaders with correct unit constructors - Update catalogs.py, pos_server, comet_catalog, callbacks, gen_images - Change mag column type from NUMERIC to TEXT (was already storing JSON) - Remove dead format_size_value() from utils.py Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Store size as raw arcsecond array instead of verbose object - Fix case-sensitive filename for EGC catalog data Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ages - Draw size ellipse/polygon overlay on POSS images using SizeObject extents - Add NSEW cardinal direction labels with correct rotation and flip/flop - Import position_angle from Steinicke data into SizeObject - Apply telescope flip_image/flop_image to catalog images - Add image_nsew and image_bbox user preferences with menu toggles - Guard size overlay against fov <= 0 Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Extract cardinal_vectors() and size_overlay_points() from get_display_image - Consolidate flip/flop into fx/fy multipliers (no more if-branches) - Add 19 unit tests covering rotation, flip/flop, scaling, orthogonality Co-Authored-By: Claude Opus 4.6 <[email protected]>
mrosseel
changed the title
…zation - Extract parse_arcmin_size to catalog_import_utils (was duplicated in caldwell_loader and specialized_loaders) - Remove dead filter_size/UNKNOWN_SIZE/_calc_filter_size from SizeObject - Compact JSON keys: extents->e, pa->p - Remove try/except fallback around SizeObject.from_json in catalogs.py - Delete 5 commented-out const assignments across loaders - Fix herschel_loader crash on missing NGC objects - Rebuild DB with new format Co-Authored-By: Claude Opus 4.6 <[email protected]>
DB was built with SQLite 3.51 which produced a format unreadable by the Pi's SQLite 3.34. Rebuilt with Python 3.9 venv (SQLite 3.45) and checkpointed WAL to ensure portability. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The objects DB is read-only at runtime, so WAL mode is unnecessary and creates -wal/-shm sidecars that break git-based deployment (stale sidecars corrupt the new DB). - Use WAL only during catalog import for write performance - Checkpoint and switch to DELETE mode after import - Remove WAL pragma from ObjectsDatabase.__init__ Co-Authored-By: Claude Opus 4.6 <[email protected]>
The rotation matrix aligned the semi-major axis with +X, but PA=0 means aligned with North which is 90° offset. Subtract 90° from the overlay rotation angle to match cardinal_vectors convention. Add test asserting PA=0 major axis aligns with North at all rotations. Co-Authored-By: Claude Opus 4.6 <[email protected]>
POSS images from SkyView have East-left (standard astronomical convention). cardinal_vectors had East negated, causing E/W label swap and PA applied N-through-W instead of N-through-E. Fixes: - cardinal_vectors: e = (-fx*cos, -fy*sin) instead of (fx*cos, fy*sin) - size_overlay_points: theta = image_rotate - pa - 90 (was + pa) - Adds test_pa90_aligns_with_east to verify PA=90° aligns with East Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Show only topmost + leftmost cardinal labels to reduce clutter and avoid bottom area where eyepiece text lives - Fix file handle leak in load_sharpless (descriptions file never closed) - Replace print with logging.debug in load_barnard - Remove SizeObject.strip() shim, use str() in object_list caller Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
DB conflict resolved by taking upstream (now includes Lynga + Harris
catalogs). Sizes branch's DB rebuild + WAL removal commits are
superseded by upstream's newer DB. Old SizeObject-encoded entries are
gone, but SizeObject.from_json('') returns an empty SizeObject so this
should be backward-compatible at runtime.
Completes the vertex rendering path that the SizeObject API has exposed since a4d5ce2. Draws connected polylines (e.g. asterism chains like "Kemble's Cascade") for SizeObjects whose extents are [ra, dec] vertex pairs, on both the catalog detail image and the live chart. - cat_images.py: vertex_overlay_points() projects [ra, dec] vertex pairs to pixel coords via gnomonic projection. The size-overlay branch dispatches on size.is_vertices and draws a polyline. - plot.py: StarField.project_vertices() does the same projection using the existing skyfield/pandas pipeline used for star markers. - ui/chart.py: collects vertex-mode targets and observing-list objects, draws polylines via project_vertices. - tests/test_cat_images.py: 12 new tests covering SizeObject vertex mode (from_vertices, is_vertices, max_extent_arcsec, JSON round trip) and the vertex_overlay_points projection (center, offset, scaling, count). Also makes SizeObject/MagnitudeObject.from_json tolerate the legacy plain-text encoding the upstream DB still ships (catalog loads gracefully against an unrebuilt DB; a re-import populates proper extents/mags). Segment-mode (geometry="segments") rendering is intentionally out of scope here — the SizeObject API supports it but no catalog currently emits segment-mode data.
CI mypy hit 10 errors: - composite_object.py: SizeObject.extents is a sum type (numeric extents vs vertex pairs vs segment pairs). The runtime narrows via is_vertices / is_segments but mypy can't follow it. Add `cast()` calls inside the branches that need the narrower type, plus an explicit annotation on the local `verts` accumulator. - catalog_imports/lynga_loader.py: utils.format_size_value was removed in the SizeObject refactor; the lynga loader still called it. Build a SizeObject directly via from_arcmin instead. No behavior change.
Crash on pifinder-mr2 when opening object details: TypeError: argument of type 'SizeObject' is not iterable at object_details.py: 'if "x" in self.object.size'. The contrast reserve calculation still treated CompositeObject.size as a string (substring checks for 'x' / "'", .replace, .split). Use SizeObject.extents (already in arcseconds) directly. Skip vertex / segment geometries since those don't represent simple diameters.
Crash on pifinder-mr2: ValueError: could not convert string to float: '' when opening object details for an object whose mag_str is empty. The previous guard only checked for '-'; legacy DB rows can have empty mag_str. Parse mag_str up front and skip the contrast-reserve calculation if it isn't a number.
mag_str is the *display* string ('-', '12.5', '12.5/13.5', or '');
parsing it for numeric work fails on multi-mag rows like '12.5/13.5'
even when the previous TypeError/ValueError guard catches the empty
case.
Use the structured numeric field self.object.mag.filter_mag instead,
treating MagnitudeObject.UNKNOWN_MAG (99) as 'no magnitude available'.
# Conflicts: # python/PiFinder/cat_images.py
… report parse_arcmin_size now strips ', ", and ° suffixes, treats x/×/+ and / as axis separators, and routes arcsecond tokens through from_arcsec instead of arcmin. Previously these were dropped (Taas200, Caldwell) or silently truncated to a single wrong axis, and EGC's arcsecond sizes (e.g. 36") were both dropped and would have been 60x inflated. Adds count_nonempty_sizes_per_catalog() to print_database() so each regen reports per-catalog objects-with-extents coverage — an empty SizeObject still serialises to valid JSON, so a presence check alone misses this regression. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
EGC size-extent coverage 4% -> 67% (arcsecond values now parsed at correct scale), Ta2 98% -> 100%; all other catalog counts unchanged, integrity ok. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
mrosseel
added a commit
to mrosseel/PiFinder
that referenced
this pull request
Jun 12, 2026
mrosseel
added a commit
to mrosseel/PiFinder
that referenced
this pull request
Jun 12, 2026
The squash-merge of brickbots#393 included the Image... settings menu; the merge of upstream/main re-inserted our copy beside it because surrounding menu entries had shifted. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
brickbots
added a commit
that referenced
this pull request
Jun 12, 2026
* feat: introduce SizeObject for structured angular sizes Replace plain string size field with SizeObject class that stores angular extents in arcseconds, mirroring the MagnitudeObject pattern. - SizeObject stores extents internally in arcseconds with constructors from_arcmin, from_arcsec, from_degrees for type-safe unit handling - JSON serialization (to_json/from_json) for DB storage - Auto-selects display unit (°, ', ") based on magnitude - filter_size field (max extent in arcminutes) enables future filtering - Variable-length extents list: 1=circle, 2=ellipse, N=polygon - Update all 12 catalog loaders with correct unit constructors - Update catalogs.py, pos_server, comet_catalog, callbacks, gen_images - Change mag column type from NUMERIC to TEXT (was already storing JSON) - Remove dead format_size_value() from utils.py Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: simplify SizeObject JSON to plain array, fix EGC.tsv case - Store size as raw arcsecond array instead of verbose object - Fix case-sensitive filename for EGC catalog data Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add size overlay, NSEW labels, and position angle to catalog images - Draw size ellipse/polygon overlay on POSS images using SizeObject extents - Add NSEW cardinal direction labels with correct rotation and flip/flop - Import position_angle from Steinicke data into SizeObject - Apply telescope flip_image/flop_image to catalog images - Add image_nsew and image_bbox user preferences with menu toggles - Guard size overlay against fov <= 0 Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: extract overlay math into testable helpers with unit tests - Extract cardinal_vectors() and size_overlay_points() from get_display_image - Consolidate flip/flop into fx/fy multipliers (no more if-branches) - Add 19 unit tests covering rotation, flip/flop, scaling, orthogonality Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: deduplicate size parsing, remove dead code, compact serialization - Extract parse_arcmin_size to catalog_import_utils (was duplicated in caldwell_loader and specialized_loaders) - Remove dead filter_size/UNKNOWN_SIZE/_calc_filter_size from SizeObject - Compact JSON keys: extents->e, pa->p - Remove try/except fallback around SizeObject.from_json in catalogs.py - Delete 5 commented-out const assignments across loaders - Fix herschel_loader crash on missing NGC objects - Rebuild DB with new format Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: rebuild DB with compatible SQLite version DB was built with SQLite 3.51 which produced a format unreadable by the Pi's SQLite 3.34. Rebuilt with Python 3.9 venv (SQLite 3.45) and checkpointed WAL to ensure portability. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: remove WAL mode from runtime, finalize DB at import The objects DB is read-only at runtime, so WAL mode is unnecessary and creates -wal/-shm sidecars that break git-based deployment (stale sidecars corrupt the new DB). - Use WAL only during catalog import for write performance - Checkpoint and switch to DELETE mode after import - Remove WAL pragma from ObjectsDatabase.__init__ Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: correct 90° offset in size overlay ellipse rotation The rotation matrix aligned the semi-major axis with +X, but PA=0 means aligned with North which is 90° offset. Subtract 90° from the overlay rotation angle to match cardinal_vectors convention. Add test asserting PA=0 major axis aligns with North at all rotations. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: correct East vector sign and PA direction in overlay POSS images from SkyView have East-left (standard astronomical convention). cardinal_vectors had East negated, causing E/W label swap and PA applied N-through-W instead of N-through-E. Fixes: - cardinal_vectors: e = (-fx*cos, -fy*sin) instead of (fx*cos, fy*sin) - size_overlay_points: theta = image_rotate - pa - 90 (was + pa) - Adds test_pa90_aligns_with_east to verify PA=90° aligns with East Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: show only 2 cardinal labels, fix file leak and cleanup - Show only topmost + leftmost cardinal labels to reduce clutter and avoid bottom area where eyepiece text lives - Fix file handle leak in load_sharpless (descriptions file never closed) - Replace print with logging.debug in load_barnard - Remove SizeObject.strip() shim, use str() in object_list caller Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: extend SizeObject with vertex/segment geometry support Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: render SizeObject vertex polylines for asterisms Completes the vertex rendering path that the SizeObject API has exposed since a4d5ce2. Draws connected polylines (e.g. asterism chains like "Kemble's Cascade") for SizeObjects whose extents are [ra, dec] vertex pairs, on both the catalog detail image and the live chart. - cat_images.py: vertex_overlay_points() projects [ra, dec] vertex pairs to pixel coords via gnomonic projection. The size-overlay branch dispatches on size.is_vertices and draws a polyline. - plot.py: StarField.project_vertices() does the same projection using the existing skyfield/pandas pipeline used for star markers. - ui/chart.py: collects vertex-mode targets and observing-list objects, draws polylines via project_vertices. - tests/test_cat_images.py: 12 new tests covering SizeObject vertex mode (from_vertices, is_vertices, max_extent_arcsec, JSON round trip) and the vertex_overlay_points projection (center, offset, scaling, count). Also makes SizeObject/MagnitudeObject.from_json tolerate the legacy plain-text encoding the upstream DB still ships (catalog loads gracefully against an unrebuilt DB; a re-import populates proper extents/mags). Segment-mode (geometry="segments") rendering is intentionally out of scope here — the SizeObject API supports it but no catalog currently emits segment-mode data. * fix: type-narrow SizeObject Union[List[float], List[List[float]]] paths CI mypy hit 10 errors: - composite_object.py: SizeObject.extents is a sum type (numeric extents vs vertex pairs vs segment pairs). The runtime narrows via is_vertices / is_segments but mypy can't follow it. Add `cast()` calls inside the branches that need the narrower type, plus an explicit annotation on the local `verts` accumulator. - catalog_imports/lynga_loader.py: utils.format_size_value was removed in the SizeObject refactor; the lynga loader still called it. Build a SizeObject directly via from_arcmin instead. No behavior change. * fix: convert object_details size handling to SizeObject API Crash on pifinder-mr2 when opening object details: TypeError: argument of type 'SizeObject' is not iterable at object_details.py: 'if "x" in self.object.size'. The contrast reserve calculation still treated CompositeObject.size as a string (substring checks for 'x' / "'", .replace, .split). Use SizeObject.extents (already in arcseconds) directly. Skip vertex / segment geometries since those don't represent simple diameters. * fix: handle empty/non-numeric mag_str in object_details Crash on pifinder-mr2: ValueError: could not convert string to float: '' when opening object details for an object whose mag_str is empty. The previous guard only checked for '-'; legacy DB rows can have empty mag_str. Parse mag_str up front and skip the contrast-reserve calculation if it isn't a number. * fix: use MagnitudeObject.filter_mag for contrast calculation mag_str is the *display* string ('-', '12.5', '12.5/13.5', or ''); parsing it for numeric work fails on multi-mag rows like '12.5/13.5' even when the previous TypeError/ValueError guard catches the empty case. Use the structured numeric field self.object.mag.filter_mag instead, treating MagnitudeObject.UNKNOWN_MAG (99) as 'no magnitude available'. * Regenerate objects database after upstream merge * fix(catalog): parse unit-suffixed/separated size tokens; add coverage report parse_arcmin_size now strips ', ", and ° suffixes, treats x/×/+ and / as axis separators, and routes arcsecond tokens through from_arcsec instead of arcmin. Previously these were dropped (Taas200, Caldwell) or silently truncated to a single wrong axis, and EGC's arcsecond sizes (e.g. 36") were both dropped and would have been 60x inflated. Adds count_nonempty_sizes_per_catalog() to print_database() so each regen reports per-catalog objects-with-extents coverage — an empty SizeObject still serialises to valid JSON, so a presence check alone misses this regression. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Regenerate objects database with fixed size parser EGC size-extent coverage 4% -> 67% (arcsecond values now parsed at correct scale), Ta2 98% -> 100%; all other catalog counts unchanged, integrity ok. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix: correct overlay rotation sense and NESW label placement PIL's Image.rotate() turns counterclockwise, but the overlay math (cardinal vectors, size ellipse, vertex outline) assumed clockwise. The two agree at roll=0, so it looked right in tests, but at any other roll the ellipse was off by 2x roll — perpendicular near 45°. Negate image_rotate in all three helpers. NESW labels now show the leftmost and rightmost cardinals at the FOV ring, brighter (128) and clamped below the titlebar text so both letters are always visible. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix: drop Image menu block duplicated by upstream merge The squash-merge of #393 included the Image... settings menu; the merge of upstream/main re-inserted our copy beside it because surrounding menu entries had shifted. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * refactor: single rotation_radians helper, typed overlay signatures Hoist the PIL-rotates-counterclockwise negation (and its explanation) into one rotation_radians() helper instead of repeating it in each overlay function, and add type hints to the overlay helpers. Also update the two tests that still encoded the old (inverted) rotation convention. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * docs: document object-image overlays and the Settings Image... menu Describe the NSEW cardinal letters and the object-size outline in the user guide's Object Images section (with a doc-styled M104 screenshot captured from the fixed rendering), and add the Image... menu to the menu map's Settings tree. Co-Authored-By: Claude Fable 5 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Richard <[email protected]>
brickbots
added a commit
to mrosseel/PiFinder
that referenced
this pull request
Jun 13, 2026
Resolves conflicts in favor of main's final SizeObject/overlay implementation (brickbots#393, brickbots#468) which subsumes this branch's earlier sizes work; keeps the branch's gen_images.py rewrite (main's small fix targeted code the rewrite removed). Co-Authored-By: Claude Fable 5 <[email protected]>
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.
Summary
SizeObjectfor structured angular sizes: stores extents in arcseconds with position angle, replaces raw string/float size fields across all catalog loadersfilter_size(max extent in arcmin)Test plan
pytest -m unit— all tests pass including 19 new overlay math tests🤖 Generated with Claude Code