-
Notifications
You must be signed in to change notification settings - Fork 19
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
Refactor UQ tests #126
Comments
@mjwen Can you tell me which file you are referring to? |
My bad, forget to insert links. I was referring to test files in this directory: https://github.com/openkim/kliff/tree/master/tests/uq |
I assume you are referring to, for example, lines 28--79 in test_mcmc.py? Those lines are for getting data to use in mcmc test. I can see it now that there are a lot of overlaps in the setup in test_mcmc.py and test_bootstrap_empirical.py. When I wrote these tests, I wrote them independently. But I can see that we can put all these setups into one file. If I put them in conftest.py, is there anything else I need to change in the test scripts? Or do I just need to import conftest? |
Great! You don't import conftest, but directly use the function names defined in conftest. You put them in a conftest.py file in the UQ folder, and then use fixture to define them. For example, @pytest.fixture()
def func1():
return "aha" Then in your actual tests you can pass the function name defined in conftest.py ( test_a_feacture(func1):
# here func1 will be "aha" Note, this will only be effective for functions defined with For example, I have defined |
I see. This is a new thing for me. Thanks for telling me about it. I will work on this if you want. I might be slow, though. |
Thanks! No need to rush at all. Take your time. |
Hi @yonatank93, in #125 I have refactored the tests to use more recent techniques in pytest to make it easier to maintain and write new tests.
I've slightly tweaked the UQ tests to make it work. But it would be great if you can make a bit more updates.
Let me know if it is unclear and I'd be happy to explain more.
The text was updated successfully, but these errors were encountered: