Reintroduce skimage.test utility by grlee77 · Pull Request #5909 · scikit-image/scikit-image

grlee77 · 2021-10-18T19:33:01Z

Description

resolves #3569 by reintroducing skimage.test. Users can run tests via

import skimage
skimage.test()

or for a specific modules with more verbose output:

import skimage
skimage.test(tests=['skimage.color', 'skimage.exposure'], verbose=2)

I had one test failure locally in a viewer plugin test that involved the camera image. The issue seems to be that these tests are getting skipped on CI so we never updated the expected value when we updated the camera image.

As suggested by @hmaarrfk in #3569, GHA should run the tests via skimage.test() for the sdist test case.

Checklist

Docstrings for all functions
Gallery example in ./doc/examples (new features only)
Benchmark in ./benchmarks, if your changes aren't covered by an
existing benchmark
Unit tests
Clean style in the spirit of PEP8
Descriptive commit messages (see below)

For reviewers

Check that the PR title is short, concise, and will make sense 1 year
later.
Check that new functions are imported in corresponding __init__.py.
Check that new features, API changes, and deprecations are mentioned in
doc/release/release_dev.rst.

Prior to the conversion to pytest users could import skimage and then run the tests via skimage.test(). This PR restores that behavior, copying an existing approach based on a PytestTester class from NumPy. Tests for a single submodule can be run using: skimage.test(['skimage.color'])

This test case does not seem to have been updated when we updated the camera image

…age.test()

pep8speaks · 2021-10-18T19:33:05Z

Hello @grlee77! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file skimage/_shared/testing.py:

Line 29:1: E402 module level import not at top of file

Comment last updated at 2021-10-19 01:45:26 UTC

stefanv

Thanks, Greg, this is perfect.

skimage/_shared/testing.py

Without this change, 'python setup.py sdist' fails with a circular import unless _remap.pyx has already been compiled

stefanv · 2021-10-18T20:22:33Z

Nice, that last commit will be helpful for the lazy loading PR as well!

grlee77 · 2021-10-18T20:25:54Z

Now it is failing because we don't require pytest in our runtime requirements, but pytest._shared.testing.py does require it.

We could move PytestTester to a different location, or I can see about delaying the import of pytest in that file.

stefanv · 2021-10-18T20:33:29Z

Can we just do def test(): ... and import it in there?

…ytest add git date and hash to __version__ in dev builds

grlee77 · 2021-10-18T20:42:29Z

Can we just do def test(): ... and import it in there?

The issue was the following lines predating this PR:

scikit-image/skimage/_shared/testing.py

Lines 24 to 34 in d44ceda

    
           import pytest 
        
           from ._warnings import expected_warnings 
        
           SKIP_RE = re.compile(r"(\s*>>>.*?)(\s*)#\s*skip\s+if\s+(.*)$") 
        
           skipif = pytest.mark.skipif 
        
           xfail = pytest.mark.xfail 
        
           parametrize = pytest.mark.parametrize 
        
           raises = pytest.raises 
        
           fixture = pytest.fixture

We only ever import from skimage._shared.testing from within test_*.py functions. In this PR, I was instead importing from that file in __init__.py which lead to the problem. I just moved the tester class to a separate tester.py file to get around that.

grlee77 · 2021-10-18T21:07:26Z

MacOS failures look unrelated:

Warning, treated as error:
/Users/runner/work/scikit-image/scikit-image/doc/source/index.rst:14:toctree contains reference to nonexisting document 'api/api'

update: I suspect that this is related given that other recent PRs passed. I suspect the change to __version__ broke the doc build because now I see the following in the log:
*WARNING* API documentation not generated: Installed version does not match source version

stefanv · 2021-10-19T22:01:21Z

Looks good to me; @hmaarrfk ?

hmaarrfk · 2021-10-19T22:05:57Z

skimage/__init__.py

+                [tag for tag in __version__.split('+')
+                 if not tag.startswith('git')]
+            )
+            __version__ += f'+git{git_date}.{git_hash}'


Do we not want to use this as our new version? This is strange reporting different versions for different ways of testing the code.

Yes, that's probably a good idea.

na, this is ok. this checks for dev in the version. I think it is fine.

hmaarrfk · 2021-10-19T22:30:43Z

Thanks!

grlee77 added 3 commits October 18, 2021 15:03

Update expected result in viewer test

6c384b0

This test case does not seem to have been updated when we updated the camera image

update tools/github/script.sh to run tests in the sdist case via skim…

7ed6e02

…age.test()

grlee77 added 🔧 type: Maintenance Refactoring and maintenance of internals ⏩ type: Enhancement Improve existing features labels Oct 18, 2021

grlee77 requested review from hmaarrfk and stefanv October 18, 2021 19:33

grlee77 added 2 commits October 18, 2021 15:35

pep8

2636ddb

fix circular import. mark late import with #noqa for pep8speaks

0bd4179

stefanv approved these changes Oct 18, 2021

View reviewed changes

skimage/_shared/testing.py Outdated Show resolved Hide resolved

move import of _remap compiled function into map_array

16a8831

Without this change, 'python setup.py sdist' fails with a circular import unless _remap.pyx has already been compiled

move PytestTester out of testing.py to avoid runtime requirement on p…

fa10fb8

…ytest add git date and hash to __version__ in dev builds

fix version comparison during doc build

b08da10

stefanv approved these changes Oct 19, 2021

View reviewed changes

hmaarrfk reviewed Oct 19, 2021

View reviewed changes

hmaarrfk merged commit b2901bf into scikit-image:main Oct 19, 2021

grlee77 mentioned this pull request Oct 29, 2021

Lazily load data_dir into the top-level namespace #5996

Merged

grlee77 mentioned this pull request Feb 20, 2022

2021's calendar of community management #5169

Closed

jarrodmillman mentioned this pull request Feb 4, 2023

Use test that can fail for sdist #6731

Merged

agriyakhetarpal mentioned this pull request Jan 25, 2025

Ensure pytest configuration in pyproject.toml is picked up #7670

Closed

Uh oh!

Conversation

grlee77 commented Oct 18, 2021

Description

Checklist

For reviewers

Uh oh!

pep8speaks commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2021-10-19 01:45:26 UTC

Uh oh!

stefanv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stefanv commented Oct 18, 2021

Uh oh!

grlee77 commented Oct 18, 2021

Uh oh!

stefanv commented Oct 18, 2021

Uh oh!

grlee77 commented Oct 18, 2021

Uh oh!

grlee77 commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefanv commented Oct 19, 2021

Uh oh!

hmaarrfk Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

stefanv Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

hmaarrfk Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

hmaarrfk commented Oct 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pep8speaks commented Oct 18, 2021 •

edited

Loading

grlee77 commented Oct 18, 2021 •

edited

Loading