Skip to content

Build tools: don't use the pytest.fixtures decorator anymore in class fixtures #3458

Merged
jni merged 1 commit intoscikit-image:masterfrom
hmaarrfk:cleanup_testing_warnings
Nov 19, 2018
Merged

Build tools: don't use the pytest.fixtures decorator anymore in class fixtures #3458
jni merged 1 commit intoscikit-image:masterfrom
hmaarrfk:cleanup_testing_warnings

Conversation

@hmaarrfk
Copy link
Copy Markdown
Member

@hmaarrfk hmaarrfk commented Oct 9, 2018 •
edited
Loading

as explained by a pytest warning

RemovedInPytest4Warning: Fixture "setUp" called directly. Fixtures are not meant to be called directly, are created automatically when test functions request them as parameters. See https://docs.pytest.org/en/latest/fixture.html for more information.

If you can make sense of why this works, you are a hero. I can't, but it works.

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.
  • Consider backporting the PR with @meeseeksdev backport to v0.14.x

@codecov-io

This comment has been minimized.

Sign in to view
@hmaarrfk
Copy link
Copy Markdown
Member Author

hmaarrfk commented Oct 9, 2018

Don't merge just yet, why did this not get triggered: https://codecov.io/gh/scikit-image/scikit-image/pull/3458/changes

@soupault
Copy link
Copy Markdown
Member

soupault commented Oct 23, 2018 •
edited
Loading

If you can make sense of why this works, you are a hero. I can't, but it works.

This seems related - https://docs.pytest.org/en/latest/nose.html?highlight=setUp#unsupported-idioms-known-issues .

@soupault soupault added the 🔧 type: Maintenance Refactoring and maintenance of internals label Oct 23, 2018
@hmaarrfk
Copy link
Copy Markdown
Member Author

hmaarrfk commented Oct 23, 2018

@soupault so because we subclassed unittest.TestCase, we don't need the decorator? I see.

stefanv
stefanv approved these changes Oct 24, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

This looks OK to me. We could probably consider refactoring to remove the test suite classes, and then using the fixtures directly, but this works.

soupault
soupault reviewed Oct 29, 2018
View reviewed changes
skimage/measure/tests/test_marching_cubes.py Outdated
mesh_surface_area, correct_mesh_orientation)
from skimage._shared import testing
from skimage._shared.testing import assert_array_equal
from skimage._shared._warnings import expected_warnings
Copy link
Copy Markdown
Member

@soupault soupault Oct 29, 2018

Choose a reason for hiding this comment

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

Could you revert this commit, please? Otherwise, good-to-go.

Copy link
Copy Markdown
Member Author

@hmaarrfk hmaarrfk Nov 16, 2018

Choose a reason for hiding this comment

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

done!

@soupault soupault self-assigned this Oct 29, 2018
@soupault soupault added this to the 0.15 milestone Oct 29, 2018
@hmaarrfk hmaarrfk force-pushed the cleanup_testing_warnings branch from fd373ab to fc07227 Compare November 16, 2018 10:24
@hmaarrfk
TST: don't use the pytest.fixtures decorator anymore in class fixture…
c0c2c5d 
…s as explained by a pytest warning

RemovedInPytest4Warning: Fixture "setUp" called directly. Fixtures are 
not meant to be called directly, are created automatically when test 
functions request them as parameters. See 
https://docs.pytest.org/en/latest/fixture.html for more information.
@hmaarrfk hmaarrfk force-pushed the cleanup_testing_warnings branch from fc07227 to c0c2c5d Compare November 16, 2018 10:32
@hmaarrfk
Copy link
Copy Markdown
Member Author

hmaarrfk commented Nov 16, 2018

@jni, this is the PR i mentioned that lets the tests pass.

This was referenced Nov 16, 2018
Merged
Closed
@hmaarrfk hmaarrfk changed the title TST: don't use the pytest.fixtures decorator anymore in class fixtures as explained by a pytest warning TST: don't use the pytest.fixtures decorator anymore in class fixtures Nov 16, 2018
@hmaarrfk
Copy link
Copy Markdown
Member Author

hmaarrfk commented Nov 18, 2018

@sciunto this fixes #3557

@jni jni merged commit 9b083fd into scikit-image:master Nov 19, 2018
@sciunto
Copy link
Copy Markdown
Member

sciunto commented Nov 19, 2018

oh, thanks!

@jni
Copy link
Copy Markdown
Member

jni commented Nov 19, 2018

Thanks @hmaarrfk, sorry I missed this the first time around!

@sciunto
Copy link
Copy Markdown
Member

sciunto commented Nov 19, 2018

same for me :)

@jni As indicated in #3555 we'll need to backport this PR I believe.

@jni
Copy link
Copy Markdown
Member

jni commented Nov 19, 2018

@sciunto do I look like Meeseeks? LOL

@meeseeksdev backport to v0.14.x

meeseeksmachine pushed a commit to meeseeksmachine/scikit-image that referenced this pull request Nov 19, 2018
@jni
Backport PR scikit-image#3458: TST: don't use the pytest.fixtures dec…
d473f06 
…orator anymore in class fixtures
@meeseeksmachine meeseeksmachine mentioned this pull request Nov 19, 2018
Merged
@sciunto
Copy link
Copy Markdown
Member

sciunto commented Nov 19, 2018

:D :D :D

sciunto added a commit that referenced this pull request Nov 19, 2018
@sciunto
Merge pull request #3558 from meeseeksmachine/auto-backport-of-pr-345…
1da9be6 
…8-on-v0.14.x

Backport PR #3458 on branch v0.14.x (TST: don't use the pytest.fixtures decorator anymore in class fixtures )
@hmaarrfk hmaarrfk changed the title TST: don't use the pytest.fixtures decorator anymore in class fixtures Build tools: don't use the pytest.fixtures decorator anymore in class fixtures Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soupault soupault soupault left review comments

@stefanv stefanv stefanv approved these changes

Assignees

@soupault soupault

Labels

🔧 type: Maintenance Refactoring and maintenance of internals

Projects

None yet

Milestone

0.15

Development

Successfully merging this pull request may close these issues.

6 participants

@hmaarrfk @codecov-io @soupault @sciunto @jni @stefanv