Follow-up to #842, deliberately deferred from it.
Why
After #842, src/core/python/ contains no package source. Everything in it is wheel machinery:
|
|
pyproject.toml.in, setup.py, MANIFEST.in |
classic wheel build |
generate_stubs.py, stubgen_pyproject.toml |
stub generation |
8× requirements*.txt |
dependency source of truth |
CMakeLists.txt |
staging install rule, python_wheel target, stubgen |
(setup.py and generate_stubs.py are .py, but build tools — neither ships.)
Two problems with leaving it:
src/python/ and src/core/python/ now mean opposite things — package source vs wheel machinery. Same name, one nested under the other's parent. A newcomer greps one and finds the other.
- It sits under
src/core/, which is otherwise a list of C++ modules.
Proposal
Move the directory wholesale to src/python/packaging/.
Preferred over a repo-root packaging/ because it packages src/python/isaacteleop/ — locality is real — and it keeps everything Python-distribution-related under one root, matching the repo convention of sources under src/.
Move as a unit. Splitting would strand the requirements*.txt away from the pyproject.toml.in that reads them via file = [...].
Safe from leaking into either wheel, verified: the staging glob is scoped to isaacteleop/*.py (src/python/CMakeLists.txt:36) and wheel.packages maps only isaacteleop (pyproject.toml:135), so a sibling directory under src/python/ cannot enter the staged tree or either wheel.
Blast radius — smaller than it looks
15 references to src/core/python outside itself. 13 are prose comments; only these are load-bearing:
src/viz/python/CMakeLists.txt:43 — ${CMAKE_SOURCE_DIR}/src/core/python/stubgen_pyproject.toml
src/viz/python/CMakeLists.txt:47 — ${CMAKE_SOURCE_DIR}/src/core/python/generate_stubs.py
src/core/CMakeLists.txt:45 — add_subdirectory(python)
Worth noting the comment hits in .github/workflows/build-ubuntu.yml:100 and .github/actions/setup-v2d-src/action.yml:8 are explanatory text, not paths — an earlier estimate treated them as CI breakage risk and over-scoped the change on that basis.
The 13 comments should still be updated, in pyproject.toml (3), src/viz/python/CMakeLists.txt (2), src/python/CMakeLists.txt (2), src/core/retargeting_engine_tests/python/pyproject.toml (2), and one each in src/core/AGENTS.md, scripts/setup_v2d_src.sh, src/core/retargeting_engine_tests/python/CMakeLists.txt, and the two CI files above.
Why it was deferred
Bisectability. Bundling a directory rename with #842's ~95-file move means a CI failure can't be attributed to either half.
Verification
Same gates as #842 — the change should be provably inert:
- staged tree (
python_package/<config>/isaacteleop/) byte-identical by sha256
- classic wheel members and hashes unchanged
cmake --install --component isaacteleop_wheel still yields zero .py
ctest green
Follow-up to #842, deliberately deferred from it.
Why
After #842,
src/core/python/contains no package source. Everything in it is wheel machinery:pyproject.toml.in,setup.py,MANIFEST.ingenerate_stubs.py,stubgen_pyproject.tomlrequirements*.txtCMakeLists.txtpython_wheeltarget, stubgen(
setup.pyandgenerate_stubs.pyare.py, but build tools — neither ships.)Two problems with leaving it:
src/python/andsrc/core/python/now mean opposite things — package source vs wheel machinery. Same name, one nested under the other's parent. A newcomer greps one and finds the other.src/core/, which is otherwise a list of C++ modules.Proposal
Move the directory wholesale to
src/python/packaging/.Preferred over a repo-root
packaging/because it packagessrc/python/isaacteleop/— locality is real — and it keeps everything Python-distribution-related under one root, matching the repo convention of sources undersrc/.Move as a unit. Splitting would strand the
requirements*.txtaway from thepyproject.toml.inthat reads them viafile = [...].Safe from leaking into either wheel, verified: the staging glob is scoped to
isaacteleop/*.py(src/python/CMakeLists.txt:36) andwheel.packagesmaps onlyisaacteleop(pyproject.toml:135), so a sibling directory undersrc/python/cannot enter the staged tree or either wheel.Blast radius — smaller than it looks
15 references to
src/core/pythonoutside itself. 13 are prose comments; only these are load-bearing:src/viz/python/CMakeLists.txt:43—${CMAKE_SOURCE_DIR}/src/core/python/stubgen_pyproject.tomlsrc/viz/python/CMakeLists.txt:47—${CMAKE_SOURCE_DIR}/src/core/python/generate_stubs.pysrc/core/CMakeLists.txt:45—add_subdirectory(python)Worth noting the comment hits in
.github/workflows/build-ubuntu.yml:100and.github/actions/setup-v2d-src/action.yml:8are explanatory text, not paths — an earlier estimate treated them as CI breakage risk and over-scoped the change on that basis.The 13 comments should still be updated, in
pyproject.toml(3),src/viz/python/CMakeLists.txt(2),src/python/CMakeLists.txt(2),src/core/retargeting_engine_tests/python/pyproject.toml(2), and one each insrc/core/AGENTS.md,scripts/setup_v2d_src.sh,src/core/retargeting_engine_tests/python/CMakeLists.txt, and the two CI files above.Why it was deferred
Bisectability. Bundling a directory rename with #842's ~95-file move means a CI failure can't be attributed to either half.
Verification
Same gates as #842 — the change should be provably inert:
python_package/<config>/isaacteleop/) byte-identical by sha256cmake --install --component isaacteleop_wheelstill yields zero.pyctestgreen