Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compatibility with new pymatgen #206

Merged
merged 6 commits into from
Oct 22, 2024
Merged

Conversation

kavanase
Copy link
Contributor

@kavanase kavanase commented Oct 22, 2024

Hi!
The latest pymatgen release breaks for pymatgen-analysis-defects (and all tests in doped/ShakeNBreak as a result) due to a name change in pymatgen.analysis.local_env. This fixes it.

Traceback:
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/site-packages/_pytest/python.py:493: in importtestmodule
    mod = import_path(
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/site-packages/_pytest/pathlib.py:582: in import_path
    importlib.import_module(module_name)
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/importlib/__init__.py:90: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1387: in _gcd_import
    ???
<frozen importlib._bootstrap>:1360: in _find_and_load
    ???
<frozen importlib._bootstrap>:1331: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:935: in _load_unlocked
    ???
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:184: in exec_module
    exec(co, module.__dict__)
tests/test_analysis.py:20: in <module>
    from test_thermodynamics import custom_mpl_image_compare
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:184: in exec_module
    exec(co, module.__dict__)
tests/test_thermodynamics.py:24: in <module>
    from doped.generation import _sort_defect_entries
doped/generation.py:20: in <module>
    from pymatgen.analysis.defects import core, thermo
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/site-packages/pymatgen/analysis/defects/core.py:19: in <module>
    from .utils import get_plane_spacing
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/site-packages/pymatgen/analysis/defects/utils.py:21: in <module>
    from pymatgen.analysis.local_env import cn_opt_params
E   ImportError: cannot import name 'cn_opt_params' from 'pymatgen.analysis.local_env' (/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/site-packages/pymatgen/analysis/local_env.py)

@kavanase
Copy link
Contributor Author

These tests are failing because the GH Actions uses the pymatgen-analysis-defects strict installation which installs pymatgen==2024.5.1. With the current version all tests pass for me locally.
Would be much appreciated if this could be merged and released soon!

@tschaume tschaume merged commit 18dcfb6 into materialsproject:main Oct 22, 2024
5 checks passed
@kavanase
Copy link
Contributor Author

Thanks very much for this @tschaume!
Not so urgent, but just to flag in case it's gone missing with the materialsproject team, there is a deprecation issue with mp-pyrho (which this package uses) which I PR'd a fix for here: materialsproject/pyrho#120

@tschaume
Copy link
Member

Thanks @kavanase for the headsup! I'll take a look at the pyrho PR.

A new version v2024.10.22 for the defects addon is up on PyPI.

@DanielYang59
Copy link

DanielYang59 commented Nov 2, 2024

I'm really sorry for this change. I didn't realize this recording would be needed externally, I would add it back (and unit test).

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.

3 participants