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

Functionals tests are rather opaque #34

Closed
bwohlberg opened this issue Oct 8, 2021 · 3 comments
Closed

Functionals tests are rather opaque #34

bwohlberg opened this issue Oct 8, 2021 · 3 comments
Assignees
Labels
tests Pertaining to SCICO tests

Comments

@bwohlberg
Copy link
Collaborator

The high level structure of scico.tests.test_functional.py is quite difficult to follow, in part due to the large number of helper functions, which make it difficult to identify the overall structure the main entry points for the tests. It would benefit greatly from some additional comments to explain the structure and identify the main components.

@bwohlberg bwohlberg added the tests Pertaining to SCICO tests label Oct 8, 2021
@Michael-T-McCann
Copy link
Contributor

Michael-T-McCann commented Jan 14, 2022

From discussion w/ Luke: the fixtures exist because it was hard to parametrize over dtype. It also avoids instantiating a LinOp repeatedly.

@lukepfister
Copy link
Contributor

lukepfister commented Jan 14, 2022

So, to summarize:

The use of fixtures is so that you can choose a dtype and instantiate test variables only once. As far as I can remember, you cant use a pytest.parameterize on a test class __init__ method.

This isn't a big deal for the functional tests; you could certainly swap out the fixtures for parameterized tests. The fixture approach was first used in the LinOp tests because LinOp construction is rather slow, and we didn't want to be re-instantiating LinOps for each test.

I'm happy to walk through how the fixtures work if it would be helpful, or you can see here

@Michael-T-McCann
Copy link
Contributor

Moved to the internal list of long-term projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Pertaining to SCICO tests
Projects
None yet
Development

No branches or pull requests

3 participants