feat(glyphs): expose TexturedGlobeGlyph's tilt transform for callers - #323
Merged
Conversation
Add rotation_matrix(spin) and transform(points, spin) so a caller can place their own scene geometry consistently with the rendered globe instead of reimplementing the tilt. The methods return / apply the exact body-to-world transform the glyph uses -- R_tilt(x) @ R_z(spin), i.e. spin about the polar axis then the axial tilt about world x -- and _spun_mesh now goes through rotation_matrix, so the exposed transform provably lands where the mesh does. - transform() accepts a (3,) point or an (N, 3) array in the body frame (unit sphere, +z pole, equatorial plane z=0) and returns the world-space position. - The X-axis tilt default is unchanged; drop the now-redundant cached _tilt_matrix (rotation_matrix is the single source of truth). - Add tests (transform lands where the mesh does, matrix = tilt then spin, shape/validation, default-tilt mesh unchanged) and a docs example. Closes #322
- transform() checks ndim before indexing shape[-1], so a scalar / 0-d input raises the documented ValueError instead of IndexError (M1); add scalar cases to the bad-shape test. - Wrap the rotation_matrix / transform returns in np.asarray so the declared ndarray return type is concrete, clearing two new no-any-return mypy errors under warn_return_any (M2). - Annotate transform's points as numpy.typing.ArrayLike (it accepts array-likes such as a plain list) (N2) and document that non-finite points propagate (N1).
…ring) Add the missing 'from cleopatra.basemap.reference import relief' so the snippet runs as copy-pasted (L2), and widen the axis limits after plotting -- draw() pins them to the unit sphere, so the radius-1.3 geostationary ring was clipped outside the [-1, 1] cube (L3).
Assert the behaviours the code supports but nothing pinned down (100% coverage could not catch a regression in them): empty (0,3) and (1,3) shapes preserved (not squeezed), list/array-like input, rotation_matrix orthogonality + freshness (a returned matrix a caller mutates must not corrupt a later call), transform usable before _prepare/draw, and the returned array not aliasing the input. Strengthen the array test to assert the transform is applied per row (N3).
Reword the docs prose to describe the transform as spin-then-tilt (R_tilt @ R_z), matching rotation_matrix's docstring, so the two don't read as opposite orders (N1); note that inf inputs to transform also emit a numpy RuntimeWarning (N2).
Construct the glyph before the pytest.raises block so only the transform() call -- the one meant to fail -- can throw inside it.
|
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
1 participant
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.
Description
Exposes the transform
TexturedGlobeGlyphapplies to place its sphere, so a caller can align their ownscene geometry with the rendered globe instead of reimplementing the tilt (implements #322; found
adopting the glyph in earthlens'
eclipse_geometry_3d.ipynb, which draws an eclipse marker, ageostationary ring, and a Moon orbit plane that must all sit correctly relative to the globe).
rotation_matrix(spin=0.0)— the(3, 3)body-to-world matrix the glyph uses:R_tilt(x) @ R_z(spin)(spin about the polar axis, then the fixed axial tilt about world
x). Computable without a texture/draw.transform(points, spin=0.0)— applies it to a(3,)point or(N, 3)array of body-frame points(unit sphere,
+zat the north pole, equatorial planez = 0,(lon, lat)→[cos(lat)·cos(lon), cos(lat)·sin(lon), sin(lat)]) and returns the world-space positions._spun_meshnow goes throughrotation_matrix, and the redundant cached_tilt_matrixis dropped — so the exposed transform provably lands where the glyph's own mesh does._spun_meshstill yieldsR_x(tilt_deg) @ R_z(spin) @ basefor every existing caller. (Per the issue's "Out of Scope", the defaultis untouched — this exposes option B, not a new tilt axis.)
No new runtime dependencies.
Issues
Type of change
Check relevant points.
How Has This Been Tested?
Two
/review-roundspasses were run and every finding resolved (a scalar-inputValueErrorfix, concrete return types clearing the new mypy errors, anArrayLikeannotation, and contract tests for empty/1x3/list input,rotation_matrixorthogonality+freshness, pre-_prepareuse, and output independence). SonarCloud quality gate is OK (0 open issues).New
TestTiltTransformcases intests/test_textured_globe_glyph.py(86 tests total; 100% line + branchcoverage of the module) covering the issue's Definition of Done:
transform(..., spin)equals_spun_mesh(spin)(the core DoD guarantee).rotation_matrixis tilt-then-spin — equalsR_x(tilt) @ R_z(spin); identity attilt=0, spin=0.transformshapes/values — a(3,)point →(3,)(north pole under a 90° tilt →-y);(N, 3)→(N, 3);bad shapes (
(2,),(4, 2), 3-D) raiseValueError.R_x(tilt) @ R_z(spin) @ base(backward-compat regression).Reproduce (external uv env, worktree on
src):tests/test_textured_globe_glyph.py(86 passed, 100% coverage)pytest tests/ -q→ 2558 passed;ruff check+ruff format --checkclean;module doctests pass
Checklist: