refactor: fix correctness bugs, add tests, and document the package#1
Merged
Conversation
The package worked for the happy path but had several defects that only appeared at the edges, no CI on commits, and a README that stopped at a single example. This reworks all three while keeping the 2.0.x API working. Correctness fixes: - Target(target_value=0) was discarded. The default was chosen with `target_value or <bound>`, so a falsy 0 became an unreachable bound and the session could never stop on that target. - last_value raised AttributeError before the first round, and after an interrupt during it. It is now derived from the persisted history, so it is also correct straight after a restart. - Target() was a mutable default argument shared by every trainer. - train()/predict before compile() raised AttributeError on an internal attribute; they now raise RuntimeError explaining what to call. - A target naming an absent metric raised a bare KeyError; it now lists the available keys and points at validation_data for val_* names. - Checkpoint parent directories were not created, so nested paths failed. - Restored values stayed as 0-d object arrays instead of floats/lists. - requires-python said >=3.9, but TensorFlow has needed >=3.10 since 2.16. - Dropped the unused polars dependency. Structure and tooling: - Split into target.py, trainer.py and _storage.py; public imports unchanged. - Renamed InfinityTraining -> InfiniteTrainer and optimize_* -> best_*, with the old names kept as aliases that emit DeprecationWarning until 3.0.0. - Added a CI workflow running lint, tests on 3.10-3.12 and a packaging check on every push and PR. Tests previously ran only when a release was cut. - The publish workflow now verifies the tag matches the packaged version. - Moved run.py to examples/mnist.py so it no longer ships in the wheel. Tests and docs: - 55 tests, up from 3, covering targets, stop conditions, persistence and resume, error handling, and the deprecation shims. - Rewrote the README with an API reference, a resume guide, a timeout caveat and a checkpoint security note; added CHANGELOG.md and CONTRIBUTING.md. Verified: 55 passed, ruff check and format clean, twine check passed, and the README quick-start and resume flows run as written.
Raising target-version to py310 enabled B905, and zip() over weight lists is exactly where an unnoticed length mismatch would matter, so pass strict=True rather than silencing it. Also import Callable from collections.abc.
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.
Review, refactor and documentation pass over the package. Backward compatible with 2.0.0 — existing code keeps working and warns where a name has changed. Ships as 2.1.0.
Why
The happy path worked, but a few edge cases were broken, tests only ran when a release was published, and the README stopped after one example. This addresses all three.
Correctness fixes
target_value or <bound>Target(target_value=0)silently became an unreachable bound, so training could never stop on that targetNonelast_valueonly assigned inside the training loopAttributeErrorbefore the first round, after an interrupt during it, and on every resumed sessiontarget: Target = Target()Nonesentinelcompile()AttributeErroron an internal attributeRuntimeErrornaming the missing callKeyErrorRuntimeErrorlisting available keys, hinting atvalidation_dataforval_*float/ plain weight listsrequires-python = ">=3.9">=3.10polarsdependencyStructure
target.py,trainer.py,_storage.py. Public imports unchanged.InfinityTraining→InfiniteTrainer,optimize_*→best_*. Old names, attributes, constructor keywords andpredict_optimize()all still work and emitDeprecationWarninguntil 3.0.0 — pinned bytests/test_deprecations.py.run.py→examples/mnist.py, so it no longer ships inside the wheel.logging;Ctrl+Cis no longer swallowed silently.CI
Tests previously ran only on release publication, so nothing gated a commit. Added
ci.yml: ruff lint + format, pytest on Python 3.10/3.11/3.12, and a build +twine check, on every push and PR. The publish workflow now also verifies the release tag matches the packaged version.Tests
55, up from 3 — targets and comparison rules, stop conditions, best-weight tracking, persistence and resume across sessions, error handling, and the deprecation shims.
Docs
README rewritten with a quick start, a "how it works" walkthrough, a resume guide, a full API reference, a timeout-granularity caveat and a security note about pickled checkpoints. Added
CHANGELOG.mdandCONTRIBUTING.md.Verification
The wheel contains only
infinite_training/— notests, noexamples. The README quick start and resume snippets were run verbatim: the quick start reaches its 1e-4 target after 25 rounds, and a fresh trainer on the same paths reportsrounds_completed == 25withlast_valueandbest_valuerestored.Notes for review
target_value=0change andlast_valuereturningNoneinstead of raising; both are in the changelog.requires-pythonnow excludes 3.9. Worth a sanity check that you are happy dropping a version that could not have had a working TensorFlow anyway.