Repository navigation
BUGFIX: Remove unconditional kimpy import from kim.py - #238
Merged
mjwen merged 2 commits intoOct 29, 2026
Merged
Conversation
kimpy is an optional dependency and kim.py already imports it inside a try/except that sets kimpy_avail, which KIMModel and KIMComputeArguments check before use. A bare top-level `import kimpy` added in 614c1b9 made that guard ineffective: without kimpy installed, `import kliff.models` (and everything that imports it, e.g. kliff.models.lennard_jones, kliff.models.parameter, kliff.trainer.kim_trainer) fails with ModuleNotFoundError: No module named 'kimpy'. Drop the stray import so kliff works without kimpy again and KIMModel reports the missing package via report_import_error() when used.
The math markup in three docstrings contains backslash sequences that are not valid string escapes (\sigma, \propto, \exp), which Python 3.12 and later report as SyntaxWarning when the modules are compiled. Make the two \sigma docstrings raw strings, and double the backslashes in mcmc.py, as the same line already does for \\theta. The docstring text is unchanged.
mjwen
self-requested a review
October 29, 2026 01:10
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.
Summary
kliff/models/kim.pyimports kimpy twice: once guarded (lines 21-27,try: import kimpy ... kimpy_avail = True / except ImportError: kimpy_avail = False) and once unguarded at the top (line 9,import kimpy). The unguarded one was added in 614c1b9 ("Working KIM trainer module") and is in v1.0.0, v1.0.1 and current main.testextra, see kimpy is missing from install_requires #80), but the bare import defeats thekimpy_availguard: without kimpy,import kliff.modelsfails withModuleNotFoundError: No module named 'kimpy', and so does everythingthat goes through
kliff/models/__init__.py(e.g.kliff.models.lennard_jones,kliff.models.parameter,kliff.trainer.kim_trainer,kliff.cmdline.model), even for users who never touch KIM models.kliff.modelsimports, uses kimpy at module level; all other uses are inside methods that are reached only after thekimpy_availcheck, soKIMModel(...)without kimpy now raises the intendedImportError: To useKIMModel,kimpyis need. Please install it first.Additional dependencies introduced (if any)
None.
Testing
Python 3.12.13, 3.13.14 and 3.14.3 on Windows, each in a fresh venv with numpy, scipy, ase, loguru, monty, pyyaml, requests, pytest and WITHOUT kimpy, run with
python -Band__pycache__cleared.The C++ extensions could not be built in this environment (MSVC rejects the variable-length array in
kliff/neighbor/neighbor_list.cpp), so the four extension modules only -kliff.neighbor.neighlist,kliff.legacy.descriptors.symmetry_function.sf,kliff.legacy.descriptors.bispectrum.bsandkliff.transforms.configuration_transforms.graphs.graph_module- were replaced by empty modules insys.modulesbefore importing kliff. kimpy was not stubbed except in the last check below.Before (main, df2d4de), all three versions:
After, all three versions:
Second commit: invalid escape sequences in docstrings
The math markup in three docstrings uses backslash sequences that are not valid string escapes, so Python 3.12 and later emit
SyntaxWarning: invalid escape sequencewhen the modules are compiled:kliff/utils.py:185(stress_to_voigt):\sigmakliff/dataset/configuration.py:431(Configuration.stresssetter):\sigmakliff/uq/mcmc.py:271:\proptoand\exp(the same line already writes\\theta)The two
\sigmadocstrings become raw strings; inmcmc.pythe backslashes are doubled, matching the existing\\theta. The docstring text is unchanged: every string constant in the three files is identical before and after (compared withast). Compiling all ofkliff/gives 3 SyntaxWarnings before and 0 after, on Python 3.12.13, 3.13.14 and 3.14.3.