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

Lookup MPRester API key in settings if None provided as arg #3004

Merged
merged 3 commits into from
May 21, 2023

Conversation

ml-evs
Copy link
Contributor

@ml-evs ml-evs commented May 21, 2023

Summary

Both the new and legacy versions of MPRester accepted api_key=None to be passed as an argument, triggering a fallback to the settings version of the key. The new behavior of the dynamic MPRester is to attempt to use the None value directly, i.e., without checking for an externally configured value.

This PR simply adjusts the behavior so that api_key=None will still trigger a settings lookup.

@janosh janosh enabled auto-merge (squash) May 21, 2023 22:26
@janosh janosh added fix Bug fix PRs api Application programming interface labels May 21, 2023
@janosh
Copy link
Member

janosh commented May 21, 2023

For the new Rester, reading the API key from .pmgrc.yaml or env variable required passing no args, i.e.

- MPRester(None)
+ MPRester()

Thanks for making the upper case work as well @ml-evs!

@janosh janosh merged commit 1039c70 into materialsproject:master May 21, 2023
@ml-evs ml-evs deleted the ml-evs/mp_rester_compat branch May 22, 2023 10:50
lbluque pushed a commit to lbluque/pymatgen that referenced this pull request May 23, 2023
…rialsproject#3004)

* Lookup `MPRester` API key in settings if `None` provided as arg

* shorten MPRester doc str

* add MPResterOldTest.test_api_key_is_none()

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Application programming interface fix Bug fix PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants