Test the Python floor the package claims instead of only asserting it - #75
Merged
Conversation
pyproject.toml says requires-python >=3.10 while both CI jobs pinned 3.14, so nothing ever proved the claim. Competitors run this on their own machines, where 3.10 is still common, and that is the version they would find out about the hard way. The test job becomes a matrix over 3.10 and 3.14. Lint stays on 3.14 alone, since ruff does not care which interpreter runs it. fail-fast is off so one version failing does not hide the other. Two things the matrix needed. The coverage artifact name carries the version, because two jobs uploading the same artifact name is an error rather than a merge. The Codecov upload runs from 3.14 only, so there is one coverage report rather than two racing. Verified with the exact commands CI runs, not with uv: a 3.10 venv, pip install -r requirements-dev.txt, then pytest with BPC_RUN_SLOW_TESTS=1. 60 passed, both golden masters included. That mattered, because pip on 3.10 resolves numpy 2.2.6 and scipy 1.15.3 rather than the newer pair 3.14 gets, and the baselines were generated on the newer one. The trajectories stay inside the tolerance across both, which is also a reasonable sign that the 3% is calibrated rather than lucky. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
This was referenced Jul 28, 2026
Merged
thc1006
added a commit
that referenced
this pull request
Jul 28, 2026
The two degenerate cases put their point opposite the middle of the other segment, so the clip on that branch had nothing to do and deleting it left the answer unchanged. Measured: removing either one passed everything. Both now also project past each end, where an unclipped projection runs along the infinite line to the point itself and returns zero. Degenerate B is the same gap on the other side, which the review that found the first one did not mention. The parallel fixture used 0.7 times direction_a, which left the denominator at 4.55e-13 against a 1e-12 threshold: a margin of 2.2, resting on how the multiplication and subtraction round rather than on the segments being parallel. #75 has just put this suite on a second Python and a second numpy. Identical directions cancel to exactly zero. One correction to that review: it reports np.dot and @ giving 4.55e-13 and 6.82e-13 for the same fixture. Measured here, both give 4.547474e-13. The margin was the real problem; the reduction primitive was not. Verified by mutation: removing either clip fails, and never classifying anything as parallel still fails four, so the new fixture reaches the branch. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
thc1006
added a commit
that referenced
this pull request
Jul 28, 2026
Review found a false green I introduced in this branch, and it is the same class of bug the branch exists to fix. uv picks its interpreter from .python-version, which pins 3.14, and it does not care what setup-python put on PATH. So `uv sync --locked` in the leg named 3.10 built a 3.14 environment and passed, and the floor #75 exists to test would have stopped being tested by anything while the run stayed green. Verified locally: `.python-version` is 3.14, uv run gives 3.14.5, and UV_PYTHON=3.10 gives 3.10.20. UV_PYTHON is set from the matrix now, and the job checks the interpreter it actually got rather than trusting the variable, so this cannot go quiet again. Tests run with --no-sync. `uv run` re-resolves before running unless told not to, which would have put what is tested back outside the lockfile's hands one line after pinning it. The early-warning job kept only 3.14, which is half of what the old pip matrix covered. pip does not resolve the same stack on each: the lockfile itself carries numpy 2.2.6 below Python 3.11 and 2.4.5 above it. Both legs again. It also called itself the README pip path and was not. The README gives competitors `requirements.txt`, which installs ActiveRocketPy as a built wheel; this installs `requirements-dev.txt`, which is editable, so it cannot see packaging failures. Named for what it is instead, with the gap written down rather than papered over. Also: dropped the pip cache, which was keyed on uv.lock and cached nothing that mattered once the install moved to uv; both jobs now print the versions they resolved, which is what #89 cost time for the lack of; and CONTRIBUTING no longer tells contributors to run a CI that stopped existing in this commit. The contract tests were weaker than they looked, and the review named each hole. continue-on-error is required to be the literal boolean now, since `${{ false }}` parses as a non-empty string and passed a truthiness check while GitHub evaluated it to false. The early-warning job has to actually invoke pytest. Commands are read line by line with comments stripped, so a mention in a comment cannot satisfy an assertion. And the interpreter check asserts, rather than merely reads, the version: an adversarial pass caught that one still surviving. Verified by mutation, all six caught: dropping UV_PYTHON, dropping --no-sync, making the early-warning job blocking by expression, deleting its test step, deleting its 3.10 leg, and gutting the interpreter check. Closes #92 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
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.
Takes the Python floor item from #64.
pyproject.tomlsaysrequires-python = ">=3.10"while both CI jobs pinned 3.14, so nothing ever proved the claim. Competitors run this on their own machines, where 3.10 is still common, and that is the version they would find out about the hard way. ActiveRocketPy already runs its own CI on 3.10, so the physics stack is covered there.Of the two options in #64 this is the one that keeps the promise rather than withdrawing it, since the promise turns out to be true.
What changed
The test job becomes a matrix over 3.10 and 3.14. Lint stays on 3.14 alone, since ruff does not care which interpreter runs it.
fail-fastis off so one version failing does not hide the other.Two things the matrix needed:
Verification
I ran the exact commands CI runs, not
uv. A 3.10 venv,pip install -r requirements-dev.txt, thenpytest tests/ --cov=BalloonPoppingGymEnvwithBPC_RUN_SLOW_TESTS=1. 60 passed, both golden masters included.Using the real install path mattered. pip on 3.10 resolves numpy 2.2.6 and scipy 1.15.3 rather than the newer pair 3.14 gets, and the baselines were generated on the newer one, so a solver difference could have pushed the trajectories past the tolerance. It did not. That is also a reasonable sign the 3% floor in #47 and #49 is calibrated rather than lucky, since it now holds across two scipy versions.
uv lock --checkpasses on 3.10 as well, so the step #62 added stays green on both legs.Cost
The test job runs twice. They run in parallel so wall clock is roughly unchanged; it is about double the compute minutes for that job.
Leaves the
pyproject.tomlversion bump in #64 alone, since that one is the release name and yours to set.