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

Migrate to new MP API, fix failing examples #61

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented Dec 4, 2024

  • Migrate import of MPRester from legacy pymatgen.ext.matproj to mp_api, also add mp_api to dependency install step, as previous example implicitly depends on either legacy or new MPRester but doesn't install it (MPRester would be None).
  • Bump pre-commit hooks
  • Clean up test.yml
  • Fix repo secret PMG_MAPI_KEY config, need maintainer help, [Dev/CI] Repo secret PMG_MAPI_KEY missing or set to legacy key? #62
  • Update install command to pip install -U pymatgen 2a986bd:

    matgenb/README.md

    Lines 82 to 83 in 53abce7

    # Uncomment the subsequent lines in this cell to install dependencies for Google Colab.
    # !pip install pymatgen==2022.2.27
  • Remove from __future__ import annotations and mypy from pre-commit 1c4c9f9 because notebooks are not annotated

Fix failing examples

  • 2018-03-09-Computing the Reaction Diagram between Two Compounds (neither new/legacy API worked)
  • 2019-03-11-Interface Reactions
  • 2023-03-10-Plotting a Pourbaix Diagram-new
  • 2023-12-31-FHI-aims-example

Taken over or tracked elsewhere

Examples only work with legacy MP API key

  • Might need to skip tests using legacy MP API keys only (or check for replacement)?
  • 2017-12-15-Plotting a Pourbaix Diagram

@@ -5,5 +5,5 @@ nbmake
pymatgen
pymatgen-analysis-diffusion
mp-api
# BoltzTraP2 # Does not work because numpy not detected.
Copy link
Contributor Author

@DanielYang59 DanielYang59 Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was related to a build time isolation issue from BoltzTraP2 side, have been fixed now

@DanielYang59 DanielYang59 force-pushed the fix-bs-plot-example branch 3 times, most recently from 023b792 to 2ec494d Compare December 4, 2024 05:23
@DanielYang59

This comment was marked as outdated.

@DanielYang59 DanielYang59 changed the title Fix 2017-09-03-Analyze and plot band structures Migrate to new MP API, fix failing examples Dec 4, 2024
micromamba activate venv
cd notebooks
pwd
pytest --ignore-glob=*notest.ipynb --nbmake .
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: either use notest pattern to ignore file, or insert pytest.skip mark (not sure how to achieve this for ipynb though)

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.

1 participant