Make the layup verbs callable from the package namespace - #558
Merged
Conversation
Appendix B of the methods paper says a script can import layup and call the verbs by the same names the command line uses. It could not: __init__.py exported only __version__, so layup.orbitfit() raised AttributeError. All six functions exist, but only as layup.<module>.<function>. The obvious fix does not work. Each verb shares its name with its module, and layup.orbitfit has long meant the MODULE: test_nongrav_auto, test_fit_outcome, test_iod_picker and test_iod_auto all reach do_fit, create_empty_result and _select_nongrav_auto through it, and 92 places import siblings as `from layup.convert import convert`. Rebinding the name to the function fails 11 tests in test_nongrav_auto alone. A lazy PEP 562 __getattr__ does not work either, for a subtler reason: importing any verb imports its siblings -- layup.orbitfit pulls in layup.convert -- and that binds the submodule here under the same name, so the hook is never consulted for it. layup.convert came back as the module. So the module is made callable and answers to both. layup.orbitfit(...) runs the fit; layup.orbitfit.do_fit still resolves. The lookup sits in __getattribute__ on the package, which always runs, rather than in __getattr__, which does not. visualize_notebook shares its name with nothing and is returned as the function. Nothing is imported until a verb is asked for, so `import layup` stays at about 0.05 s against the 2 s that binding all six costs. That is what keeps `layup --help` and `layup --version` at 0.06 s, and the new test pins it. 50 passed, 1 skipped across the new test and the four files that depend on the module meaning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FWmtkZQeT8EFD4e2eovpm7
test_observations_fit_convert_predict_pipeline and test_bench_residuals_runs take ~150 s and ~145 s on an idle machine against the suite-wide 300 s pytest-timeout. On a loaded runner, with four xdist workers competing for CPU, the macOS 3.13 leg exceeds it: both failed on #558's first run and the pipeline test failed again on a re-run, while ubuntu 3.13 and both 3.14 legs passed. The timeout is what fails, not the code -- the same test takes 147.1 s on main and 149.2 s on #558, so nothing recent added the cost. The next slowest test in either file is 1.28 s, so these two dominate and both sit near the cliff. Raise their per-test budget to 600 s rather than trimming the fixtures: the work is real, a full fit/convert/predict over the demo set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
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.
Appendix B of the methods paper documents
import layupfollowed bylayup.orbitfit(...), but__init__.pyexported only__version__, so that raisesAttributeError. The six functions exist only aslayup.<module>.<function>.The obvious fix does not work.
layup.orbitfitalready means the module:test_nongrav_auto,test_fit_outcome,test_iod_pickerandtest_iod_autoreachdo_fit,create_empty_resultand_select_nongrav_autothrough it, and 92 places import siblings asfrom layup.convert import convert. Rebinding the name to the function fails 11 tests intest_nongrav_autoalone.A lazy PEP 562
__getattr__does not work either: importing any verb imports its siblings —layup.orbitfitpulls inlayup.convert— which binds the submodule under the same name, so the hook is never consulted for it.So the verb modules are made callable, and answer to both:
Nothing is imported until a verb is asked for, so
import layupstays at about 0.05 s against the 2 s that binding all six costs. That is what keepslayup --helpand--versionat 0.06 s, and the new test pins it.Full suite: 558 passed, 1 skipped.
black25.1.0 clean.The alternative is one sentence in Appendix B instead —
from layup.orbitfit import orbitfit, which is what the code already does in 92 places. Happy to go that way if reviewers prefer it to a callable module.