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

typing_extension imported at run time causing ImportError #3751

Closed
janosh opened this issue Apr 12, 2024 · 6 comments · Fixed by #3752
Closed

typing_extension imported at run time causing ImportError #3751

janosh opened this issue Apr 12, 2024 · 6 comments · Fixed by #3752

Comments

@janosh
Copy link
Member

janosh commented Apr 12, 2024

Hi! Thanks for your work on pymatgen! 😃
Just to flag, this update seems to be causing some failures on our GitHub Actions tests (for ShakeNBreak and doped) with the latest pymatgen version. I'm not sure if it just needs typing_extensions to be added to the requirements?

Run pytest -vv -m "not mpl_image_compare" tests  # all non-plotting tests
============================= test session starts ==============================
platform linux -- Python 3.11.9, pytest-8.1.1, pluggy-1.4.0 -- /opt/hostedtoolcache/Python/3.11.9/x64/bin/python
cachedir: .pytest_cache
Matplotlib: 3.8.4
Freetype: 2.6.1
rootdir: /home/runner/work/ShakeNBreak/ShakeNBreak
configfile: pyproject.toml
plugins: mpl-0.1[7](https://github.com/SMTG-Bham/ShakeNBreak/actions/runs/8663869495/job/23758934155#step:6:8).0
collecting ... collected 121 items / 1 error / 29 deselected / 92 selected

==================================== ERRORS ====================================
___________________ ERROR collecting tests/test_analysis.py ____________________
ImportError while importing test module '/home/runner/work/ShakeNBreak/ShakeNBreak/tests/test_analysis.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/_pytest/python.py:520: in importtestmodule
    mod = import_path(
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/_pytest/pathlib.py:5[8](https://github.com/SMTG-Bham/ShakeNBreak/actions/runs/8663869495/job/23758934155#step:6:9)4: in import_path
    importlib.import_module(module_name)
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1204: in _gcd_import
    ???
<frozen importlib._bootstrap>:1176: in _find_and_load
    ???
<frozen importlib._bootstrap>:1147: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:6[9](https://github.com/SMTG-Bham/ShakeNBreak/actions/runs/8663869495/job/23758934155#step:6:10)0: in _load_unlocked
    ???
/opt/hostedtoolcache/Python/3.[11](https://github.com/SMTG-Bham/ShakeNBreak/actions/runs/8663869495/job/23758934155#step:6:12).9/x64/lib/python3.11/site-packages/_pytest/assertion/rewrite.py:178: in exec_module
    exec(co, module.__dict__)
tests/test_analysis.py:[14](https://github.com/SMTG-Bham/ShakeNBreak/actions/runs/8663869495/job/23758934155#step:6:15): in <module>
    from shakenbreak import analysis
shakenbreak/analysis.py:23: in <module>
    from shakenbreak import input, io
shakenbreak/input.py:30: in <module>
    from doped.generation import DefectsGenerator, name_defect_entries
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/doped/generation.py:32: in <module>
    from pymatgen.transformations.advanced_transformations import CubicSupercellTransformation
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/pymatgen/transformations/advanced_transformations.py:36: in <module>
    from pymatgen.transformations.standard_transformations import (
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/pymatgen/transformations/standard_transformations.py:[17](https://github.com/SMTG-Bham/ShakeNBreak/actions/runs/8663869495/job/23758934155#step:6:18): in <module>
    from pymatgen.analysis.elasticity.strain import Deformation
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/pymatgen/analysis/elasticity/__init__.py:5: in <module>
    from .elastic import (
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/pymatgen/analysis/elasticity/elastic.py:21: in <module>
    from pymatgen.analysis.elasticity.stress import Stress
/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/pymatgen/analysis/elasticity/stress.py:11: in <module>
    from typing_extensions import Self
E   ModuleNotFoundError: No module named 'typing_extensions'

Originally posted by @kavanase in #3705 (comment)

@DanielYang59
Copy link
Contributor

DanielYang59 commented Apr 13, 2024

Thanks for fixing! Just tracked them down and found these were added in 2adde29 (guess we were both too distracted because there were just too many mypy errors), and wondering why unit tests didn't capture such errors (we don't explicitly install typing_extensions in our CI too)?

@janosh
Copy link
Member Author

janosh commented Apr 13, 2024

Mistakes happen. It's not a big deal especially if we fix them quickly.

We don't install typing_extensions in CI but one of our optional dependencies must. Which means it's available in our CI but not necessarily available in others CI or local machines. So we didn't see the error but #3646 by @ml-evs would have caught this which is why I'm eager to get it merged. Sadly, lots of legacy issues in the way. maybe it makes sense to split out a PR that only adds minimal dependency checking and do all the type stuff in #3646 separately.

@DanielYang59
Copy link
Contributor

DanielYang59 commented Apr 13, 2024

We don't install typing_extensions in CI but one of our optional dependencies must.

Could be (just guess, didn't check).

So we didn't see the error but #3646 by @ml-evs would have caught this which is why I'm eager to get it merged. Sadly, lots of legacy issues in the way. maybe it makes sense to split out a PR that only adds minimal dependency checking and do all the type stuff in #3646 separately.

Yes I just had a look and there are 700+ pyright errors. It's certainly too much work for a single person and I would see if I could give him a hand on that.

@janosh
Copy link
Member Author

janosh commented Apr 13, 2024

Help with fixing pyright issues would be hugely appreciated!

@ml-evs
Copy link
Contributor

ml-evs commented Jun 3, 2024

This same issue was just flagged up in the matminer CI when upgrading pymatgen to 2024.5.31 from 2024.5.1: https://github.com/hackingmaterials/matminer/actions/runs/9346511294/job/25721508373?pr=939

@DanielYang59
Copy link
Contributor

Yes we noticed that #3852 :( sorry for the overlook

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 a pull request may close this issue.

3 participants