fix: replace removed distutils.spawn with shutil.which (Python 3.12+)#908
Open
alongd wants to merge 2 commits into
Open
fix: replace removed distutils.spawn with shutil.which (Python 3.12+)#908alongd wants to merge 2 commits into
alongd wants to merge 2 commits into
Conversation
`distutils` was removed from the Python standard library in 3.12, so `from distutils.spawn import find_executable` raises ModuleNotFoundError on Python 3.14 (it only works where setuptools happens to install its distutils compatibility shim). This breaks importing `arc.main`, which in turn breaks every downstream import of ARC (e.g. the T3 test suite fails at collection with `No module named 'distutils'`). Replace the single stdlib usage with `shutil.which`, the documented drop-in for `distutils.spawn.find_executable` (both return the path to the executable on PATH, or None). `shutil` is already imported. Note: ARC's own `arc.settings.settings.find_executable` is a different function (finds the python of a named conda env) and is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ARC’s ESS executable discovery to be compatible with Python 3.12+ by removing the deprecated/removed distutils dependency and using the standard-library replacement.
Changes:
- Removed
from distutils.spawn import find_executablefromarc/main.py. - Replaced ESS executable lookups (
g03/g09/g16/qchem/orca/molpro/terachem) withshutil.which().
Comments suppressed due to low confidence (1)
arc/main.py:812
- The
else:that logs "Did not find ESS on the local machine" is currently attached toif diagnostics:(inside theif any(...)block). This causes a misleading log message when ESS executables are found butdiagnosticsis False. Theelseshould instead pair withif any(...)(and optionally simplify theany([...])expression).
terachem = shutil.which('terachem')
if terachem:
self.ess_settings['molpro'] = ['local']
if any([val for val in self.ess_settings.values()]):
if diagnostics:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… log)
Two pre-existing bugs in ARC.check_ess_settings's local-ESS scan,
surfaced by review of the surrounding block:
1. When a local TeraChem executable is found, availability was recorded
under `self.ess_settings['molpro']` instead of `['terachem']`
(copy-paste error), so TeraChem was mis-registered and Molpro could
be falsely reported as available.
2. The "Did not find ESS on the local machine" log was nested under the
`if diagnostics:` branch, so it fired as the `else` of `diagnostics`
rather than the `else` of `if any(ESS found)`. As written, whenever
ESS *were* found but `diagnostics` was False, it logged the opposite
("Did not find ESS"). Dedent the `else` to pair with the `any(...)`
check so the not-found message only fires when nothing was found.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #908 +/- ##
==========================================
- Coverage 63.15% 63.10% -0.06%
==========================================
Files 114 114
Lines 38178 38177 -1
Branches 9990 9990
==========================================
- Hits 24113 24090 -23
- Misses 11183 11199 +16
- Partials 2882 2888 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Problem
distutilswas removed from the Python standard library in 3.12, soarc/main.py'sfrom distutils.spawn import find_executableraisesModuleNotFoundErroron Python 3.14.It only appears to work in environments where setuptools happens to install its
distutilscompatibility shim — a fresh CI env without that shim fails.
Because
arc.mainis imported transitively by most of ARC (and by downstream consumers likeT3), this breaks at import/collection time. Concretely, the T3 test suite currently dies with:
(chain:
t3.common→arc.species.species→arc.main→from distutils.spawn import find_executable)Fix
Replace the single stdlib usage with
shutil.which, the documented drop-in replacement fordistutils.spawn.find_executable— both return the path to the executable found onPATH, orNone.shutilis already imported inarc/main.py, so no new import is added.The 7 affected call sites all look up ESS executables on
PATH(g03/g09/g16/qchem/orca/molpro/terachem), and the downstream truthiness checks (if g03 or g09 or g16:…) are unchanged.Scope / safety
distutilsusage anywhere inarc/*.py, and there are no otherremoved-stdlib imports — this single change makes the ARC import graph Python-3.12+/3.14 clean.
arc.settings.settings.find_executable(env_name, ...)is a different function(it locates the python interpreter of a named conda env) and is intentionally left untouched;
arc/job/adapters/ase_adapter.pyimports that settings function, not the stdlib one.🤖 Generated with Claude Code