Skip to content
New issue

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

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

Already on GitHub? # 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)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant