-
Notifications
You must be signed in to change notification settings - Fork 576
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
[MAINT] Refactor fixtures #3905
Changes from all commits
8c587b8
e59ef4c
89efd6f
6dd1961
65d952c
7e23819
4259dd8
c77863a
e9fc3a5
2989d0a
e910b67
8c3b110
e830f4b
f79c1dc
f6ff611
e0bcc40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -373,7 +373,7 @@ Pre-commit will then run all those hooks on the files you have staged for commit | |
Note that if some of those hooks fail you may have to edit some files and stage them again. | ||
|
||
Tests | ||
------ | ||
----- | ||
|
||
When fixing a bug, the first step is to write a minimal test that fails because | ||
of it, and then write the bugfix to make this test pass. | ||
|
@@ -388,26 +388,70 @@ They should run on small mocked data, cover a representative range of parameters | |
For more information about this coding approach, | ||
see `test-driven development <https://en.wikipedia.org/wiki/Test-driven_development>`_. | ||
|
||
Tests must be seeded to avoid random failures. | ||
For objects using random seeds (e.g. scikit-learn estimators), pass either | ||
a `np.random.RandomState` or an `int` as the seed. | ||
When your test use random numbers, those must be generated through: | ||
We use `pytest <https://docs.pytest.org/en/6.2.x/contents.html>`_ to run our tests. | ||
|
||
.. code-block:: python | ||
If you are not familiar with pytest, | ||
have a look at this `introductory video <https://www.youtube.com/watch?v=mzlH8lp4ISA>`_ | ||
by one of the pytest core developer. | ||
|
||
rng = np.random.RandomState(0) | ||
my_number = rng.normal() | ||
In general tests for a specific module (say `nilearn/image/image.py`) | ||
are kept in a `tests` folder in a separate module | ||
with a name that matches the module being tested | ||
(so in this case `nilearn/image/tests/test_image.py`). | ||
|
||
To check your changes worked and didn't break anything run `pytest nilearn`. | ||
When you have added a test you can check that your changes worked | ||
and didn't break anything by running `pytest nilearn`. | ||
To do quicker checks it's possible to run only a subset of tests: | ||
|
||
.. code-block:: bash | ||
|
||
pytest -v test_module.py | ||
pytest -v nilearn/module/tests/test_module.py | ||
|
||
Fixtures | ||
^^^^^^^^ | ||
|
||
If you need to do some special "set up" for your tests | ||
(for example you need to generate some data, or a NiftiImage object or a file...) | ||
you can use `pytest fixtures <https://docs.pytest.org/en/6.2.x/fixture.html>`_ | ||
to help you mock this data | ||
(more information on pytest fixtures in `this video <https://www.youtube.com/watch?v=ScEQRKwUePI>`_). | ||
|
||
Fixture are recognizable because they have a `@pytest.fixture` decorator. | ||
Fixtures that are shared by many tests modules can be found in `nilearn/conftest.py` | ||
but some fixures specific to certain modules can also be kept in that testing module. | ||
Comment on lines
+414
to
+421
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we can be more explicit in instructing to first see if any of the available fixtures can be used before writing a new one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added one sentence to make it explicit. |
||
|
||
Before adding new fixtures, first check those that exist | ||
in the test modules you are working in or in `nilearn/conftest.py`. | ||
|
||
Seeding | ||
^^^^^^^ | ||
|
||
Many tests must be seeded to avoid random failures. | ||
When your test use random numbers, | ||
Remi-Gau marked this conversation as resolved.
Show resolved
Hide resolved
|
||
you can seed a random number generator with `numpy.random.default_rng` | ||
like in the following examples: | ||
|
||
.. code-block:: python | ||
|
||
def test_something(): | ||
# set up | ||
rng = np.random.default_rng(0) | ||
my_number = rng.normal() | ||
|
||
# the rest of the test | ||
|
||
You can also use the `rng` fixture. | ||
|
||
.. code-block:: python | ||
|
||
def test_something(rng): | ||
# set up | ||
my_number = rng.normal() | ||
|
||
# the rest of the test | ||
|
||
Documentation | ||
--------------- | ||
------------- | ||
|
||
Documentation must be understandable by people from different backgrounds. | ||
The “narrative” documentation should be an introduction to the concepts of | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Remi-Gau why is it that for mni affine the fixture returns the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. General answer. I went for # global variable
X = "some value"
# function
def _x()
return X
# fixture
@pytest.mark.fixture()
def x():
return _x() When some other fixtures needed to access the value of X and change it. So to avoid having that fixture change the global (AKA "best way to make things break") I added a function as a go between. I have not done that systematically because I have not had the need for it for all of those constants. But I think that it would make sense to have the same pattern for all, to avoid this confusion. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,47 +79,191 @@ def close_all(): | |
plt.close("all") # takes < 1 us so just always do it | ||
|
||
|
||
MNI_AFFINE = np.array( | ||
[ | ||
[2.0, 0.0, 0.0, -98.0], | ||
[0.0, 2.0, 0.0, -134.0], | ||
[0.0, 0.0, 2.0, -72.0], | ||
[0.0, 0.0, 0.0, 1.0], | ||
] | ||
) | ||
# ------------------------ RNG ------------------------# | ||
|
||
|
||
def _mni_3d_img(affine=MNI_AFFINE): | ||
def _rng(): | ||
return np.random.RandomState(42) | ||
|
||
|
||
@pytest.fixture() | ||
def rng(): | ||
"""Return a seeded random number generator.""" | ||
return _rng() | ||
Remi-Gau marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
# ------------------------ AFFINES ------------------------# | ||
|
||
|
||
def _affine_mni(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does anything prevent from always using this even inside conftest? What is the benefit of still having the global? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I was thinking that the global was progressively becoming useless. will try to simplify. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually removed all constants. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But isn't that a breaking change ? I'd rather have deprecation cycles... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looking through all the files changed, all those constants only appear in tests so I don't think deprecation is necessary there. are we actually storing anything in conftest.py that is not just for testing? if so we should probably move it somewhere else |
||
"""Return an affine corresponding to 2mm isotropic MNI template. | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
return np.array( | ||
[ | ||
[2.0, 0.0, 0.0, -98.0], | ||
[0.0, 2.0, 0.0, -134.0], | ||
[0.0, 0.0, 2.0, -72.0], | ||
[0.0, 0.0, 0.0, 1.0], | ||
] | ||
) | ||
|
||
|
||
@pytest.fixture() | ||
def affine_mni(): | ||
"""Return an affine corresponding to 2mm isotropic MNI template.""" | ||
return _affine_mni() | ||
|
||
|
||
def _affine_eye(): | ||
"""Return an identity matrix affine. | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
return np.eye(4) | ||
|
||
|
||
@pytest.fixture() | ||
def affine_eye(): | ||
"""Return an identity matrix affine.""" | ||
return _affine_eye() | ||
|
||
|
||
# ------------------------ SHAPES ------------------------# | ||
|
||
|
||
def _shape_3d_default(): | ||
"""Return default shape for a 3D image. | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
return (10, 10, 10) | ||
|
||
|
||
def _shape_4d_default(): | ||
"""Return default shape for a 4D image. | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
return (10, 10, 10, 10) | ||
|
||
|
||
@pytest.fixture() | ||
def shape_3d_default(): | ||
"""Return default shape for a 3D image.""" | ||
return _shape_3d_default() | ||
|
||
|
||
@pytest.fixture() | ||
def shape_4d_default(): | ||
"""Return default shape for a 4D image.""" | ||
return _shape_4d_default() | ||
|
||
|
||
def _img_zeros(shape, affine): | ||
return Nifti1Image(np.zeros(shape), affine) | ||
|
||
|
||
def _img_ones(shape, affine): | ||
return Nifti1Image(np.ones(shape), affine) | ||
|
||
|
||
# ------------------------ 3D IMAGES ------------------------# | ||
|
||
|
||
def _img_3d_rand(affine=_affine_eye()): | ||
"""Return random 3D Nifti1Image in MNI space. | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
data = _rng().rand(*_shape_3d_default()) | ||
return Nifti1Image(data, affine) | ||
|
||
|
||
@pytest.fixture() | ||
def img_3d_rand_eye(): | ||
"""Return random 3D Nifti1Image in MNI space.""" | ||
return _img_3d_rand() | ||
|
||
|
||
def _img_3d_mni(affine=_affine_mni()): | ||
data_positive = np.zeros((7, 7, 3)) | ||
rng = np.random.RandomState(42) | ||
rng = _rng() | ||
data_rng = rng.rand(7, 7, 3) | ||
data_positive[1:-1, 2:-1, 1:] = data_rng[1:-1, 2:-1, 1:] | ||
return Nifti1Image(data_positive, affine) | ||
|
||
|
||
@pytest.fixture() | ||
def mni_affine(): | ||
"""Return an affine corresponding to 2mm isotropic MNI template.""" | ||
return MNI_AFFINE | ||
def img_3d_mni(): | ||
"""Return a default random 3D Nifti1Image in MNI space.""" | ||
return _img_3d_mni() | ||
|
||
|
||
@pytest.fixture() | ||
def mni_3d_img(): | ||
"""Fixture for a random 3D image in MNI space.""" | ||
return _mni_3d_img() | ||
def _img_3d_zeros(shape=_shape_3d_default(), affine=_affine_eye()): | ||
"""Return a default zeros filled 3D Nifti1Image (identity affine). | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
return _img_zeros(shape, affine) | ||
|
||
|
||
@pytest.fixture | ||
def img_3d_zeros_eye(): | ||
"""Return a zeros-filled 3D Nifti1Image (identity affine).""" | ||
return _img_3d_zeros() | ||
|
||
|
||
def _img_3d_ones(shape=_shape_3d_default(), affine=_affine_eye()): | ||
"""Return a ones-filled 3D Nifti1Image (identity affine). | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
return _img_ones(shape, affine) | ||
|
||
|
||
# ------------------------ 4D IMAGES ------------------------# | ||
|
||
|
||
def _img_4d_zeros(shape=_shape_4d_default(), affine=_affine_eye()): | ||
"""Return a default zeros filled 4D Nifti1Image (identity affine). | ||
|
||
Mostly used for set up in other fixtures in other testing modules. | ||
""" | ||
return _img_zeros(shape, affine) | ||
|
||
|
||
@pytest.fixture | ||
def img_4d_zeros_eye(): | ||
"""Return a default zeros filled 4D Nifti1Image (identity affine).""" | ||
return _img_4d_zeros() | ||
|
||
|
||
@pytest.fixture | ||
def img_4d_ones_eye(): | ||
"""Return a default ones filled 4D Nifti1Image (identity affine).""" | ||
return _img_ones(_shape_4d_default(), _affine_eye()) | ||
|
||
|
||
@pytest.fixture | ||
def img_4D_rand_eye(): | ||
"""Return a default random filled 4D Nifti1Image (identity affine).""" | ||
data = _rng().rand(*_shape_4d_default()) | ||
return Nifti1Image(data, _affine_eye()) | ||
|
||
|
||
@pytest.fixture() | ||
def testdata_4d_for_plotting(): | ||
"""Random 4D images for testing figures for multivolume data.""" | ||
rng = np.random.RandomState(42) | ||
img_4d = Nifti1Image(rng.uniform(size=(7, 7, 3, 10)), MNI_AFFINE) | ||
img_4d_long = Nifti1Image(rng.uniform(size=(7, 7, 3, 1777)), MNI_AFFINE) | ||
img_mask = Nifti1Image(np.ones((7, 7, 3), dtype="uint8"), MNI_AFFINE) | ||
rng = _rng() | ||
img_4d = Nifti1Image(rng.uniform(size=(7, 7, 3, 10)), _affine_mni()) | ||
img_4d_long = Nifti1Image(rng.uniform(size=(7, 7, 3, 1777)), _affine_mni()) | ||
img_mask = Nifti1Image(np.ones((7, 7, 3), dtype="uint8"), _affine_mni()) | ||
atlas = np.ones((7, 7, 3), dtype="int32") | ||
atlas[2:5, :, :] = 2 | ||
atlas[5:8, :, :] = 3 | ||
img_atlas = Nifti1Image(atlas, MNI_AFFINE) | ||
img_atlas = Nifti1Image(atlas, _affine_mni()) | ||
atlas_labels = { | ||
"gm": 1, | ||
"wm": 2, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the doc on the tests to talk about fixtures and improve this section in general with a couple of extra links.