Skip to content

BUGFIX: Remove unconditional kimpy import from kim.py - #238

Merged
mjwen merged 2 commits into
openkim:mainfrom
pattems:py313-bugfix-remove-unconditional-kimpy-import
Oct 29, 2026
Merged

mjwen merged 2 commits into
openkim:mainfrom
pattems:py313-bugfix-remove-unconditional-kimpy-import

Conversation

@pattems

@pattems pattems commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • kliff/models/kim.py imports 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.
  • kimpy is meant to be optional (it is only in the test extra, see kimpy is missing from install_requires #80), but the bare import defeats the kimpy_avail guard: without kimpy, import kliff.models fails with ModuleNotFoundError: No module named 'kimpy', and so does everything
    that 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.
  • Fix: delete the stray line. Nothing else in kim.py, or in the modules kliff.models imports, uses kimpy at module level; all other uses are inside methods that are reached only after the kimpy_avail check, so KIMModel(...) without kimpy now raises the intended ImportError: To use KIMModel, kimpy is 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 -B and __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.bs and kliff.transforms.configuration_transforms.graphs.graph_module - were replaced by empty modules in sys.modules before importing kliff. kimpy was not stubbed except in the last check below.

Before (main, df2d4de), all three versions:

import kliff.models
-> ModuleNotFoundError: No module named 'kimpy'
pytest tests/models/test_parameter.py tests/models/test_parameter_transform.py
-> 2 errors during collection (ModuleNotFoundError: No module named 'kimpy')

After, all three versions:

import kliff.models                      -> OK, kliff.models.kim.kimpy_avail == False
KIMModel("SW_StillingerWeber_1985_Si__MO_405512056662_006")
                                         -> ImportError: To use `KIMModel`, `kimpy` is need. ...
with a stub `kimpy` module in sys.modules -> OK, kimpy_avail == True
pytest tests/models/test_parameter.py tests/models/test_parameter_transform.py
                                         -> 2 passed

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 sequence when the modules are compiled:

  • kliff/utils.py:185 (stress_to_voigt): \sigma
  • kliff/dataset/configuration.py:431 (Configuration.stress setter): \sigma
  • kliff/uq/mcmc.py:271: \propto and \exp (the same line already writes \\theta)

The two \sigma docstrings become raw strings; in mcmc.py the 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 with ast). Compiling all of kliff/ gives 3 SyntaxWarnings before and 0 after, on Python 3.12.13, 3.13.14 and 3.14.3.

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
mjwen self-requested a review October 29, 2026 01:10
@mjwen
mjwen merged commit 922f2a6 into openkim:main Oct 29, 2026
0 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants