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

Can no longer import some modules without having ASE installed #3644

Closed
ml-evs opened this issue Feb 22, 2024 · 2 comments · Fixed by #3645
Closed

Can no longer import some modules without having ASE installed #3644

ml-evs opened this issue Feb 22, 2024 · 2 comments · Fixed by #3645
Labels
ase Atomic simulation environment bug serialization Data/object serialization

Comments

@ml-evs
Copy link
Contributor

ml-evs commented Feb 22, 2024

Python version

3.11.7

Pymatgen version

2024.2.20

Operating system version

Any

Current behavior

I observed this in a fresh virtual environment when trying to install and use mp_api. Before #3619 (or related follow-up commits, can't tell) you were able to import the AseAtomsAdaptor without an error, even if you didn't have ASE installed. This was changed by the creation of the MSONAtoms class which uses Atoms via inheritance and thus is loaded at import time.

Example as posted in #3619 (comment)

>>> from mp_api.client.mprester import MPRester
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-mp-api/lib/python3.11/site-packages/mp_api/client/__init__.py", line 8, in <module>
    from .mprester import MPRester
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-mp-api/lib/python3.11/site-packages/mp_api/client/mprester.py", line 10, in <module>
    from emmet.core.electronic_structure import BSPathType
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-mp-api/lib/python3.11/site-packages/emmet/core/electronic_structure.py", line 11, in <module>
    from pymatgen.analysis.magnetism.analyzer import (
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-mp-api/lib/python3.11/site-packages/pymatgen/analysis/magnetism/__init__.py", line 5, in <module>
    from pymatgen.analysis.magnetism.analyzer import (
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-mp-api/lib/python3.11/site-packages/pymatgen/analysis/magnetism/analyzer.py", line 24, in <module>
    from pymatgen.transformations.advanced_transformations import MagOrderingTransformation, MagOrderParameterConstraint
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-mp-api/lib/python3.11/site-packages/pymatgen/transformations/advanced_transformations.py", line 33, in <module>
    from pymatgen.io.ase import AseAtomsAdaptor
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-mp-api/lib/python3.11/site-packages/pymatgen/io/ase.py", line 44, in <module>
    class MSONAtoms(Atoms, MSONable):
                    ^^^^^
NameError: name 'Atoms' is not defined

Expected Behavior

The module should still be importable in cases where ase is not available, as it was before.

Minimal example

python -m venv .venv
. .venv/bin/activate
pip install pymatgen
$ python -c "from pymatgen.analysis.magnetism.analyzer import *"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-pymatgen/lib/python3.11/site-packages/pymatgen/analysis/magnetism/__init__.py", line 5, in <module>
    from pymatgen.analysis.magnetism.analyzer import (
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-pymatgen/lib/python3.11/site-packages/pymatgen/analysis/magnetism/analyzer.py", line 24, in <module>
    from pymatgen.transformations.advanced_transformations import MagOrderingTransformation, MagOrderParameterConstraint
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-pymatgen/lib/python3.11/site-packages/pymatgen/transformations/advanced_transformations.py", line 33, in <module>
    from pymatgen.io.ase import AseAtomsAdaptor
  File "/home/mevans/repos/re2fractive/re2fractive/experiments/2_featurizing_refractive_index_data/.venv-pymatgen/lib/python3.11/site-packages/pymatgen/io/ase.py", line 44, in <module>
    class MSONAtoms(Atoms, MSONable):
                    ^^^^^
NameError: name 'Atoms' is not defined

which runs correctly if you pip install pymatgen==2024.2.8.

Relevant files to reproduce this bug

No response

@ml-evs
Copy link
Contributor Author

ml-evs commented Feb 22, 2024

Also just a comment that I would've expected this to be picked up by a linter; for me Atoms is possible unbound is flagged up by pyright but perhaps mypy is less picky as configured?

@DanielYang59
Copy link
Contributor

I noticed the same issue while trying to export XRD pattern for example here with mp_api with the following demo code:

from mp_api.client import MPRester
from pymatgen.analysis.diffraction.xrd import XRDCalculator
from pymatgen.symmetry.analyzer import SpacegroupAnalyzer

with MPRester(api_key="<enter your api key>") as mpr:
    # first retrieve the relevant structure
    structure = mpr.get_structure_by_material_id("mp-728693")

# important to use the conventional structure to ensure
# that peaks are labelled with the conventional Miller indices
sga = SpacegroupAnalyzer(structure)
conventional_structure = sga.get_conventional_standard_structure()

# this example shows how to obtain an XRD diffraction pattern
# these patterns are calculated on-the-fly from the structure
calculator = XRDCalculator(wavelength="CuKa")
pattern = calculator.get_pattern(conventional_structure)

The ImportError is captured in

try:
from ase.atoms import Atoms
from ase.calculators.singlepoint import SinglePointDFTCalculator
from ase.constraints import FixAtoms
from ase.io.jsonio import decode, encode
from ase.spacegroup import Spacegroup
ase_loaded = True
except ImportError:
ase_loaded = False

So no explicit error like no module named ase is raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ase Atomic simulation environment bug serialization Data/object serialization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants