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

Fixes the problem of weird synthesis search results when querying for a range #324

Merged
merged 24 commits into from
Jul 15, 2021

Conversation

hhaoyan
Copy link
Contributor

@hhaoyan hhaoyan commented Jul 7, 2021

Fixes the problem of weird synthesis search results when querying for a range. For example, when querying for 700-1000, the old code would return documents that only match one-sided criteria.

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • Use "$elemMatch" to for AND query of array elements.
  • I have run the tests locally and they passed.
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

@hhaoyan
Copy link
Contributor Author

hhaoyan commented Jul 14, 2021

Hi @munrojm I have added tests for the data adaptors. I was trying to add tests for the synthesis API data endpoint but confused because I don't know how to test an API that interacts with the database. Any ideas or could you point me to an existing test suite?

@munrojm
Copy link
Member

munrojm commented Jul 14, 2021

Hi @hhaoyan, all you need to do is set an environment variable MP_API_KEY with the API key you obtain from https://next-gen.materialsproject.org, and MP_API_ENDPOINT with the appropriate endpoint. If you are running the app locally, this should be set to your localhost and appropriate port. Running pytest should then execute all of the tests.

You should also pull in changes from main which should let you do the above with any problems.

@hhaoyan
Copy link
Contributor Author

hhaoyan commented Jul 15, 2021

Have added necessary tests. Should be good to go!

@munrojm
Copy link
Member

munrojm commented Jul 15, 2021

Thanks @hhaoyan !

@munrojm munrojm merged commit 329740e into materialsproject:main Jul 15, 2021
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.

2 participants