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

Migrate ext.COD from mysql to REST API #4117

Merged
merged 11 commits into from
Oct 21, 2024

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented Oct 17, 2024

Summary


@pytest.mark.skipif(not which("mysql"), reason="No mysql")
def test_get_structure_by_formula(self):
Copy link
Contributor Author

@DanielYang59 DanielYang59 Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime for test_get_structure_by_formula is pretty long at around 30 seconds:

...                                                                                                                        [100%]
====================================================== slowest 30 durations ======================================================
30.34s call     tests/ext/test_cod.py::TestCOD::test_get_structure_by_formula
2.05s call     tests/ext/test_cod.py::TestCOD::test_get_cod_ids
1.94s call     tests/ext/test_cod.py::TestCOD::test_get_structure_by_id

(6 durations < 0.005s hidden.  Use -vv to show these durations.)
3 passed in 37.00s

Reason being Li2O returned 16 structures, and each take ~ 2s, maybe use FeH18C3(N3F2)3 instead which has only one match.

@DanielYang59 DanielYang59 force-pushed the migrate-ext-cod-to-rest branch 2 times, most recently from eae9a4e to 9dbf05e Compare October 17, 2024 11:40
@DanielYang59 DanielYang59 marked this pull request as ready for review October 17, 2024 15:22
@shyuep shyuep merged commit aa9bdbf into materialsproject:master Oct 21, 2024
43 checks passed
@shyuep
Copy link
Member

shyuep commented Oct 21, 2024

Thanks.

@DanielYang59
Copy link
Contributor Author

No problem, thanks for reviewing and for original bringing up the idea to migrate to REST API

@DanielYang59 DanielYang59 deleted the migrate-ext-cod-to-rest branch October 22, 2024 02:08
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.

[Dev] Remove mysql dependency from ext.cod and fix S608
2 participants