Prepare for Python 3.12 and 3.13: drop distutils, make GLT install fa… - #764
Draft
kmontemayor2-sc wants to merge 7 commits into
Draft
kmontemayor2-sc wants to merge 7 commits into
kmontemayor2-sc wants to merge 7 commits into
Conversation
…ilures fatal Python 3.12 removed `distutils` and the deprecated `unittest` aliases, and GiGL uses both. The `distutils` imports work today only because `setuptools` is installed transitively and ships a compatibility shim; a library must not depend on that. - Add `gigl.common.utils.parse.str_to_bool` and use it in place of `distutils.util.strtobool` at all 15 call sites. Accepted and rejected spellings match `strtobool`. Every call site already coerced the result with `bool()` or used it as a condition, so the `int` to `bool` return change is not observable. - Rename the 26 `assertEquals` / `assertNotEquals` uses to `assertEqual` / `assertNotEqual`, and select ruff `UP005` so they cannot return. - Make a failed `install_glt.sh` fatal. `main()` returned the child's status but the `__main__` block discarded it, so `requirements/install_py_deps.sh` saw exit 0 under `set -e` and every base image build continued after a failed GLT install. Measured against a stub that exits 7: the old script exits 0, the new one exits 7. A successful install still exits 0. - Run `ty` twice in `make type_check`, at the 3.11 floor and at 3.13. `ty` resolves the standard library against one version per invocation, so the floor pass accepts modules 3.13 removed and only the ceiling pass rejects them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
/help |
Contributor
GiGL Automation@ 23:18:23UTC : 🤖 Available PR CommandsYou can trigger the following workflows by commenting on this PR:
💡 Usage: Simply comment on this PR with any of the commands above (e.g., ⏱️ Note: Commands may take some time to complete. Progress updates will be posted as comments. |
Contributor
GiGL Automation@ 23:19:26UTC : Starting to build base images for CUDA and CPU. |
# Conflicts: # gigl/scripts/post_install.py
main added tests/unit/scripts_test/post_install_test.py, which runs the shipped post_install.py with bash shimmed on PATH and asserts that a failing and a succeeding install_glt.sh set the process exit code. That is the behaviour this branch's tests/unit/scripts/post_install_test.py checked. With main's post_install.py taken in the merge, the second file only duplicates the first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
type_check ran ty twice with hard-coded minors (3.11 from pyproject, 3.13 from the Makefile). A single PYTHON_VERSION variable now picks the interpreter for every uv command and the ty target version, so CI can run the same targets once per minor (make ... PYTHON_VERSION=3.13). PYTHON_VERSION defaults to the contents of .python-version and is exported as UV_PYTHON; uv resolves that to the same interpreter it picks from .python-version today. type_check passes the major.minor to ty, which makes [tool.ty.environment] python-version redundant, so it is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
type_check no longer has a separate 3.13 pass; the 3.13 check is make type_check PYTHON_VERSION=3.13. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The workbench Dockerfile declared ARG PYTHON_VERSION="3.9". Docker exposes a build ARG to RUN steps as an environment variable, and the Makefile now reads PYTHON_VERSION to choose the interpreter, so `make install_deps` in that image would ask uv for Python 3.9, which GiGL does not support. The conda env and GiGL now use the same supported minor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
…ilures fatal
Python 3.12 removed
distutilsand the deprecatedunittestaliases, and GiGL uses both. Thedistutilsimports work today only becausesetuptoolsis installed transitively and ships a compatibility shim; a library must not depend on that.gigl.common.utils.parse.str_to_booland use it in place ofdistutils.util.strtoboolat all 15 call sites. Accepted and rejected spellings matchstrtobool. Every call site already coerced the result withbool()or used it as a condition, so theinttoboolreturn change is not observable.assertEquals/assertNotEqualsuses toassertEqual/assertNotEqual, and select ruffUP005so they cannot return.install_glt.shfatal.main()returned the child's status but the__main__block discarded it, sorequirements/install_py_deps.shsaw exit 0 underset -eand every base image build continued after a failed GLT install. Measured against a stub that exits 7: the old script exits 0, the new one exits 7. A successful install still exits 0.tytwice inmake type_check, at the 3.11 floor and at 3.13.tyresolves the standard library against one version per invocation, so the floor pass accepts modules 3.13 removed and only the ceiling pass rejects them.Scope of work done
Where is the documentation for this feature?: N/A
Did you add automated tests or write a test plan?
Updated Changelog.md? NO
Ready for code review?: NO