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

Refresh OPTIMADE aliases and update docstrings #3447

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

ml-evs
Copy link
Contributor

@ml-evs ml-evs commented Nov 1, 2023

Summary

Apropos of nothing in particular... here is PR that makes some updates to the OPTIMADE client (and maybe starts a discussion about its future).

Major changes:

  • Refreshes the baked-in OPTIMADE providers list with the latest new APIs
  • Expands upon some of the docstrings

Happy to talk about how we could integrate the models and client at optimade-python-tools if that would be preferable. It would allow for async pulling data from all databases, pre-flight validating filters and validating responses (dangerous stuff for some databases...) and the code here could focus on just making sure the returned objects work with pymatgen. Should be releasing o-p-t v1.0 (Materials-Consortia/optimade-python-tools#1831) before the end of the year which will support pydantic v2 and hopefully have better isolation, so wouldn't require many additional deps (httpx for async requests, Lark for parsing filters: see here)

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

@janosh janosh added housekeeping Moving around or cleaning up old code/files ecosystem Concerning the larger pymatgen ecosystem labels Nov 1, 2023
@janosh
Copy link
Member

janosh commented Nov 1, 2023

Thanks @ml-evs! 👍

I expect new deps, esp. pydantic might be a blocker. We might consider a soft dependency that prompts the user to pip install pydantic when actually using functionality that requires it.

@ml-evs
Copy link
Contributor Author

ml-evs commented Nov 1, 2023

Thanks @ml-evs! 👍

I expect new deps, esp. pydantic might be a blocker. We might consider a soft dependency that prompts the user to pip install pydantic when actually using functionality that requires it.

Haha yes, after my strop about pymatgen indirectly needing pydantic a few months ago, I don't want to be blamed for reintroducing it :P Thankfully the client can be used separately from pydantic so with a bit more separation of concerns at our end we could make it a standalone installable. I'll have a think about it -- not necessary for this PR of course.

@janosh janosh merged commit bb37024 into materialsproject:master Nov 1, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ecosystem Concerning the larger pymatgen ecosystem housekeeping Moving around or cleaning up old code/files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants