docs: units of measure in every Python docstring (#90) - #169
Merged
Conversation
Systematic sweep of the .pyi stubs and the #[pyfunction]/#[pymethods] docstrings so every dimensioned argument, return value, attribute and property states its unit (SI: meters, m/s, m/s^2, radians unless the name ends in _deg; TLE fields in their native TLE units). sgp4 now says explicitly that its TEME output is meters and m/s, not the km / km/s of the canonical Vallado implementation, and lists the output shapes. Docstrings only: no code, signature or stub-type changes. The kepler class (python/src/pykepler.rs and the class kepler section of satkit.pyi) is left untouched; it is being rewritten on another branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PG969yapJ84DJceKt21Wen
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PG969yapJ84DJceKt21Wen
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
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.
Closes #90
Systematic sweep so every Python-facing docstring states the unit of measure for each argument, return value, attribute and property that has a physical dimension. Convention is SI throughout: positions in meters, velocities in m/s, accelerations in m/s², angles in radians unless the name ends in
_deg, durations in seconds (Julian dates in days), mass kg, area m², GM m³/s², density kg/m³, temperature K, F10.7 in sfu. TLE fields keep their native TLE units (degrees, revs/day, revs/day², revs/day³, 1/Earth-radii for B*).Docstrings only — no code, signature or stub-type changes, no reformatting of unrelated prose. Where a docstring already stated the unit it was left alone.
The SGP4 callout (the reporter's example)
satkit.sgp4(bothsatkit.pyiand the runtime__doc__inpysgp4.rs) now says explicitly that position and velocity are returned in meters and meters/second in the TEME frame, with a note that the canonical Vallado implementation and most other SGP4 libraries return km and km/s and that satkit converts so SGP4 output matches every other position/velocity in the library. The output shapes are listed too —(3,),(Ntime, 3),(Ntle, 3),(Ntle, Ntime, 3)— taken from the reshape logic inpysgp4.rs.Files touched (~112 docstring hunks, 18 files)
Stubs (
python/satkit/):satkit.pyi(TLE fields, sgp4, time JD/MJD/unixtime/GPS-seconds, quaternion.from_axis_angle degrees→radians fix, itrfcoord.move_with_heading return, consts.wgs84_f, satstate/propresult covariance and STM block units, propsettings abs/rel error),frametransform.pyi(state-transform Returns; the*_approxandtransform_state_approxfunctions had no Args/Returns at all),moon.pyi(moonphase ranges are degrees; illumination unitless 0–1),spaceweather.pyi(F10.7 keys in sfu).Runtime docstrings (
python/src/):pytle.rs,pysgp4.rs,pyinstant.rs,pyconsts.rs(also correctsjgm3_awhich was labelled m³/s² — it is meters),pyquaternion.rs(rotx/roty/rotz summaries said "degrees"),pysatstate.rs,pypropresult.rs,pypropsettings.rs,pypropagate.rs,pythrust.rs,pyframetransform.rs,pylpephem_sun.rs(shadowfunchad no docstring),pyspaceweather.rs. Several getters/classes that had no runtime docstring at all (satstate,propsettings,propresultgetters,time.from_gps_week_and_second) got one matching the stub.Already complete and untouched:
sun.pyi,planets.pyi,jplephem.pyi,density.pyi,utils.pyi,__init__.pyi, and their.rscounterparts;geodetic,itrfcoord(bar one return),thrust,ecomparams,satproperties,lambert,propagatestubs.Excluded:
keplerpython/src/pykepler.rsand theclass kepler:section ofsatkit.pyiare deliberately untouched (verified byte-identical tomain) — that class is being rewritten on a separate branch and edits here would only conflict.Left alone (needs a decision, not a docstring)
satkit.density.nrlmsise(altitude, latitude, longitude, time)float form has a stub-vs-runtime unit mismatch. The stub names the parameterslatitude_rad/longitude_radand documents radians, butpydensity.rspasses the floats straight tonrlmsise::nrlmsise, which takes degrees (theitrfcoordoverload callslatitude_deg()for the same slots). Fixing it means changing either the stub signature or the binding, both outside a docstring-only PR. Suggest a follow-up issue.propsettings.abs_erroris described as "in the units of the state (meters for position, m/s for velocity)" based onsettings.rs("infinity norm of the state") and the tolerance being passed through unscaled; worth a second look if the integrator scales internally.spaceweather.get()cp/kp— indices, no unit word added.Non-unit stub/runtime drifts noticed but out of scope:
pylpephem_moon.rs::phase_nameReturns saysstr|list(it returnsmoonphase);pylpephem_planets.rs::heliocentric_posArgs are in the wrong order/type vs the signature;pypropagate.rsstill names the positional argstate0where the stub saysstate.Verification
cargo fmt --all -- --check,cargo clippy --workspace --all-targets -- -D warnings: cleanpip install -e . --no-build-isolation(in a private venv, not the shared site-packages), then stubtest exactly as CI runs it (--allowlist python/stubtest_allowlist.txt --ignore-positional-only --ignore-disjoint-bases): no issues in 10 modulespytest python/test -qwithSATKIT_DATA/SATKIT_TESTVEC_ROOT: 191 passed, 2 skippedsatkit.sgp4.__doc__checked for the meters / kilometers wording🤖 Generated with Claude Code
https://claude.ai/code/session_01PG969yapJ84DJceKt21Wen