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

OPTIMADE client generates malformed queries #2852

Closed
ml-evs opened this issue Feb 20, 2023 · 5 comments · Fixed by #2853
Closed

OPTIMADE client generates malformed queries #2852

ml-evs opened this issue Feb 20, 2023 · 5 comments · Fixed by #2853

Comments

@ml-evs
Copy link
Contributor

ml-evs commented Feb 20, 2023

Describe the bug
At some point, the OPTIMADE client in this repo was linted (presumably) and f'?response_fields="{response_fields}"...' was replaced with f'?response_fields={response_fields!r}', which generates query parameters with the wrong (according to the OPTIMADE spec, at least) kind of quotes:

url = join(resource, f"v1/structures?filter={optimade_filter}&{response_fields=}")

This renders the client useless, as every request has such malformed queries.

I'm happy to fix this (will make a PR straightaway), but please let me know if this could be released before the end of the week --- if not I will have to adjust the pymatgen OPTIMADE tutorials that are being delivered at a conference on the 28th Feb.

@ml-evs
Copy link
Contributor Author

ml-evs commented Feb 20, 2023

(same thing also applies to string queries within the filter query parameter)

@janosh
Copy link
Member

janosh commented Feb 20, 2023

@ml-evs Sorry! That was my fault. We can definitely make a release before week's end. Thanks for #2853! 🙏

@ml-evs
Copy link
Contributor Author

ml-evs commented Feb 20, 2023

No problem, and thanks! Also found another bug where chemical_formula_hill was being passed incorrectly in queries (instead duplicating chemical_formula_anonymous).

My own client at https://www.optimade.org/optimade-python-tools/latest/getting_started/client/ is getting pretty mature and could probably integrated here... (already supports pymatgen Structure output, obeys all the standardized metadata we have for delaying requests, async queries over providers, custom callbacks for e.g., saving/storing in databases) Extra deps would be httpx and rich, could split it into a namespace package of optimade-python-tools if there is any interest.

@janosh
Copy link
Member

janosh commented Feb 20, 2023

My own client at https://www.optimade.org/optimade-python-tools/latest/getting_started/client/ is getting pretty mature and could probably integrated here... (already supports pymatgen Structure output, obeys all the standardized metadata we have for delaying requests, async queries over providers, custom callbacks for e.g., saving/storing in databases)

That sounds great! 👍

@janosh
Copy link
Member

janosh commented Feb 20, 2023

Extra deps would be httpx and rich, could split it into a namespace package of optimade-python-tools if there is any interest.

I recommend coordinating with @munrojm on that but I'd be happy to help/act as reviewer if we go ahead.

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.

2 participants