Make compiler and assistant proper packages - #16
Merged
Conversation
Closes #13. compiler/ had no __init__.py, so reaching it meant putting the directory on sys.path, which assistant/validator.py and tests/test_compiler.py both did. That works until it doesn't: with compiler/ on the path, compile.py is a top-level module named `compile`, shadowing the builtin for anything that imports it. Adds compiler/__init__.py and pyproject.toml. The compiler is now importable as `compiler.compile`, so the name cannot collide with the builtin, and neither consumer manipulates sys.path to find it. tests keep one sys.path entry pointing at the repo root, which is ordinary for running tests without installing, and `pip install -e .` makes even that redundant. compile.py still runs directly. It tries the package-relative import first and falls back to flat imports when executed as a script, so `python compiler/compile.py` keeps working for the four workflows, the README, and anyone with the habit. `python -m compiler.compile` and a `dlpac-compile` console script are the tidier ways in. requirements.txt now contains `-e .` rather than restating the dependencies. pyproject.toml is the single place a version is pinned, and every existing `pip install -r requirements.txt` (four workflows, README, CONTRIBUTING) installs the project without any of them needing to change. anthropic stays optional, behind the `claude` extra. The immediate benefit is distribution rather than correctness, since nothing was actually broken: an installable package is how someone tries this without cloning. Verified: compile succeeds via script, module, and console script; 13/13 compiler tests; 12/12 assistant tests; assistant dry-run eval 2/2; package imports from a directory outside the repo; `import compile` no longer resolves to this project; builtin compile() intact. Test-merged against both open PRs with no conflicts, and the combined tree passes 18/18 and 12/12.
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 #13. Reported by @jim-t-trevenx in #6.
compiler/had no__init__.py, so reaching it meant putting the directory onsys.path, which bothassistant/validator.pyandtests/test_compiler.pydid. That works until it doesn't: withcompiler/on the path,compile.pyis a top-level module namedcompile, shadowing the builtin for anything that imports it.What changed
Adds
compiler/__init__.pyandpyproject.toml. The compiler is now importable ascompiler.compile, so the name cannot collide with the builtin, and neither consumer manipulatessys.pathto find it. The tests keep a singlesys.pathentry pointing at the repo root, which is ordinary for running tests without installing, andpip install -e .makes even that redundant.Nothing that worked before stops working.
compile.pytries the package-relative import first and falls back to flat imports when executed as a script, sopython compiler/compile.pystill runs for the four workflows, the README, and anyone with the habit.python -m compiler.compileand adlpac-compileconsole script are the tidier ways in.requirements.txtnow contains-e .instead of restating dependencies.pyproject.tomlbecomes the single place a version is pinned, and every existingpip install -r requirements.txt(four workflows, README, CONTRIBUTING) installs the project without any of them needing to change.anthropicstays optional, behind aclaudeextra.Why this is worth doing
Not correctness, since nothing was broken. Distribution. An installable package is how someone tries this without cloning the repo, which is why I labelled the issue
enhancementrather thanbug.Verification
python compiler/compile.py(backwards compat)python -m compiler.compiledlpac-compile(console script)tests/test_compiler.pytests/test_assistant.pyDLPAC_BRAIN=dry-run python -m assistant.evalimport compileresolves to this projectcompile()Test-merged against both open PRs (#14, #15) with no conflicts, and the combined tree passes 18/18 compiler and 12/12 assistant tests.