Skip to content

Commit eb381e3

Browse files
committed
STYLE: Clear the review findings that were plainly correct
CONTRIBUTING.md told contributors to write Conventional Commits, but commitizen enforces the ITK prefixes that every commit here uses, so the documented rule rejected the commits it asked for. The remote-module cross-dependency warning only recognised "== X.Y.*", so a cross-dep pinned as "~= X.Y.Z" was neither rewritten nor reported. Delete the commented-out functions in wheel_builder_utils.py (push_env, debug, parse_kv_overrides, get_git_id, cmake_compiler_defaults), none of which has a caller, and hoist the function-local imports to module scope, dropping the tomli fallback that a Python 3.11+ project can never reach.
1 parent c1526bb commit eb381e3

4 files changed

Lines changed: 15 additions & 174 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,19 @@ pre-commit run --all-files
4343

4444
### Commit Messages
4545

46-
This project uses [Conventional Commits](https://www.conventionalcommits.org), enforced by Commitizen:
46+
This project uses the ITK commit message convention, `PREFIX: Description`,
47+
enforced by Commitizen:
4748

4849
```
49-
feat: add support for Python 3.12 wheels
50-
fix: correct cmake args not propagating to module builds
51-
chore: update pre-commit hook versions
52-
docs: clarify aarch64 build requirements
50+
ENH: Add support for Python 3.12 wheels
51+
BUG: Correct cmake args not propagating to module builds
52+
COMP: Update pre-commit hook versions
53+
DOC: Clarify aarch64 build requirements
5354
```
5455
55-
Commitizen will reject commits that don't follow this format.
56+
Valid prefixes are `BUG:`, `COMP:`, `DOC:`, `ENH:`, `PERF:` and `STYLE:`,
57+
and the description starts with a capital letter. Commitizen will reject
58+
commits that don't follow this format.
5659
5760
### Building Docs
5861

‎scripts/build_python_instance_base.py‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import copy
22
import os
3+
import re
34
import shutil
45
import subprocess
56
import sys
7+
import tomllib
68
from abc import ABC, abstractmethod
79
from collections import OrderedDict
810
from collections.abc import Callable
@@ -720,13 +722,6 @@ def _update_module_itk_deps(pyproject_path: Path, itk_version: str) -> bool:
720722
bool
721723
*True* if any dependency was rewritten.
722724
"""
723-
import re
724-
725-
try:
726-
import tomllib
727-
except ModuleNotFoundError:
728-
import tomli as tomllib # Python < 3.11
729-
730725
with open(pyproject_path, "rb") as f:
731726
pyproject_data = tomllib.load(f)
732727

@@ -775,7 +770,7 @@ def _update_module_itk_deps(pyproject_path: Path, itk_version: str) -> bool:
775770
# Warn about pinned remote-module cross-deps that may also need
776771
# attention but should not be auto-rewritten.
777772
cross_dep_pattern = re.compile(
778-
r'"(itk-[a-z][a-z0-9-]*)\s*==\s*[\d]+\.[\d]+\.\*"'
773+
r'"(itk-[a-z][a-z0-9-]*)\s*(?:==\s*\d+\.\d+\.\*|~=\s*\d+\.\d+(?:\.\d+)?)"'
779774
)
780775
for m in cross_dep_pattern.finditer(text):
781776
pkg = m.group(1)

‎scripts/wheel_builder_utils.py‎

Lines changed: 0 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,6 @@
1717
from os import environ
1818
from pathlib import Path
1919

20-
# @contextmanager
21-
# def push_env(**kwargs):
22-
# """This context manager allow to set/unset environment variables."""
23-
# saved_env = dict(os_environ)
24-
# for var, value in kwargs.items():
25-
# if value is not None:
26-
# os_environ[var] = value
27-
# elif var in os_environ:
28-
# del os_environ[var]
29-
# yield
30-
# os_environ.clear()
31-
# for saved_var, saved_value in saved_env.items():
32-
# os_environ[saved_var] = saved_value
33-
3420

3521
class ContextDecorator:
3622
"""A base class or mixin that enables context managers to work as
@@ -391,122 +377,6 @@ def git_describe_to_pep440(desc: str) -> str:
391377
return semver_format
392378

393379

394-
# def debug(msg: str, do_print=False) -> None:
395-
# """Print *msg* only when *do_print* is True."""
396-
# if do_print:
397-
# print(msg)
398-
#
399-
#
400-
# def parse_kv_overrides(pairs: list[str]) -> dict[str, str]:
401-
# """Parse a list of ``KEY=VALUE`` strings into a dict.
402-
#
403-
# A value of ``"UNSET"`` is stored as ``None`` so callers can remove
404-
# the key from a target mapping.
405-
#
406-
# Parameters
407-
# ----------
408-
# pairs : list[str]
409-
# Strings of the form ``KEY=VALUE``.
410-
#
411-
# Returns
412-
# -------
413-
# dict[str, str]
414-
# Parsed overrides.
415-
#
416-
# Raises
417-
# ------
418-
# SystemExit
419-
# If an entry is not a valid ``KEY=VALUE`` pair or the key name
420-
# is invalid.
421-
# """
422-
# result: dict[str, str] = {}
423-
# for kv in pairs:
424-
# if "=" not in kv:
425-
# raise SystemExit(f"ERROR: Trailing argument '{kv}' is not KEY=VALUE")
426-
# key, value = kv.split("=", 1)
427-
# if not key or not re.match(r"^[A-Za-z_][A-Za-z0-9_]*$", key):
428-
# raise SystemExit(f"ERROR: Invalid variable name '{key}' in '{kv}'")
429-
# if value == "UNSET":
430-
# # Explicitly remove if present later
431-
# result[key] = None # type: ignore
432-
# else:
433-
# result[key] = value
434-
# return result
435-
436-
437-
# def get_git_id(
438-
# repo_dir: Path, pixi_exec_path, env, backup_version: str = "v0.0.0"
439-
# ) -> str | None:
440-
# """Return a human-readable Git identifier for *repo_dir*.
441-
#
442-
# Tries, in order: exact tag, branch name, short commit hash.
443-
# Falls back to *backup_version* when none of these succeed.
444-
#
445-
# Parameters
446-
# ----------
447-
# repo_dir : Path
448-
# Root of the Git repository.
449-
# pixi_exec_path : Path or str
450-
# Path to the pixi executable (unused but kept for API compat).
451-
# env : dict
452-
# Environment variables passed to Git subprocesses.
453-
# backup_version : str, optional
454-
# Fallback identifier returned when Git queries fail.
455-
#
456-
# Returns
457-
# -------
458-
# str or None
459-
# A tag, branch name, short hash, or *backup_version*.
460-
# """
461-
# # 1. exact tag
462-
# try:
463-
# run_result = run_commandLine_subprocess(
464-
# ["git", "describe", "--tags", "--exact-match"],
465-
# cwd=repo_dir,
466-
# env=env,
467-
# check=False,
468-
# )
469-
#
470-
# if run_result.returncode == 0:
471-
# return run_result.stdout.strip()
472-
# except subprocess.CalledProcessError:
473-
# pass
474-
# # 2. branch
475-
# try:
476-
# run_result = run_commandLine_subprocess(
477-
# ["git", "rev-parse", "--abbrev-ref", "HEAD"],
478-
# cwd=repo_dir,
479-
# env=env,
480-
# )
481-
# branch = run_result.stdout.strip()
482-
# if run_result.returncode == 0 and branch != "HEAD":
483-
# return branch
484-
# except subprocess.CalledProcessError:
485-
# pass
486-
# # 3. short hash
487-
# try:
488-
# run_result = run_commandLine_subprocess(
489-
# ["git", "rev-parse", "--short", "HEAD"],
490-
# cwd=repo_dir,
491-
# env=env,
492-
# )
493-
# short_version = run_result.stdout.strip()
494-
# if run_result.returncode == 0 and short_version != "HEAD":
495-
# return short_version
496-
# except subprocess.CalledProcessError:
497-
# pass
498-
#
499-
# # 4. punt and give dummy backup_version identifier
500-
# if not (repo_dir / ".git").is_dir():
501-
# if (repo_dir / ".git").is_file():
502-
# print(
503-
# f"WARNING: {str(repo_dir)} is a secondary git worktree, and may not resolve from within dockcross build"
504-
# )
505-
# return backup_version
506-
# print(f"ERROR: {repo_dir} is not a primary git repository")
507-
# return backup_version
508-
509-
510380
def compute_itk_package_version(
511381
itk_dir: Path, itk_git_tag: str, pixi_exec_path, env
512382
) -> str:
@@ -660,30 +530,6 @@ def resolve_oci_exe(env: dict[str, str]) -> str:
660530
return "docker"
661531

662532

663-
# def cmake_compiler_defaults(build_dir: Path) -> tuple[str | None, str | None]:
664-
# info = build_dir / "cmake_system_information"
665-
# if not info.exists():
666-
# try:
667-
# out = run_commandLine_subprocess(["cmake", "--system-information"]).stdout
668-
# info.write_text(out, encoding="utf-8")
669-
# except Exception as e:
670-
# print(f"WARNING: Failed to generate cmake_system_information: {e}")
671-
# return None, None
672-
# text = info.read_text(encoding="utf-8", errors="ignore")
673-
# cc = None
674-
# cxx = None
675-
# for line in text.splitlines():
676-
# if "CMAKE_C_COMPILER == " in line:
677-
# parts = re.split(r"\s+", line.strip())
678-
# if len(parts) >= 4:
679-
# cc = parts[3]
680-
# if "CMAKE_CXX_COMPILER == " in line:
681-
# parts = re.split(r"\s+", line.strip())
682-
# if len(parts) >= 4:
683-
# cxx = parts[3]
684-
# return cc, cxx
685-
686-
687533
def give_relative_path(bin_exec: Path, build_dir_root: Path) -> str:
688534
bin_exec = Path(bin_exec).resolve()
689535
build_dir_root = Path(build_dir_root).resolve()

‎scripts/windows_build_python_instance.py‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import re
2+
import shutil
23
from pathlib import Path
34

45
from build_python_instance_base import BuildPythonInstanceBase
@@ -135,9 +136,7 @@ def build_tarball(self):
135136

136137
if seven_zip is None:
137138
# Try PATH lookup using where/which behavior from shutil
138-
import shutil as _shutil
139-
140-
found = _shutil.which("7z.exe") or _shutil.which("7z")
139+
found = shutil.which("7z.exe") or shutil.which("7z")
141140
if found:
142141
seven_zip = Path(found)
143142

@@ -169,8 +168,6 @@ def build_tarball(self):
169168

170169
# 3) Fallback: create a .zip using Python's shutil
171170
# This will create a zip archive named ITKPythonBuilds-windows.zip
172-
import shutil as _shutil
173-
174171
if out_zip.exists():
175172
try:
176173
out_zip.unlink()
@@ -179,7 +176,7 @@ def build_tarball(self):
179176
# make_archive requires base name without extension
180177
base_name = str(out_zip.with_suffix("").with_suffix(""))
181178
# shutil.make_archive will append .zip
182-
_shutil.make_archive(
179+
shutil.make_archive(
183180
base_name,
184181
"zip",
185182
root_dir=str(self.build_dir_root),

0 commit comments

Comments
 (0)