Skip to content

Casting fixture parameter to list at the beginning of parameter parsing. #5950

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 12, 2019
Merged

Casting fixture parameter to list at the beginning of parameter parsing. #5950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f639e2
Casting fixture parameter to list at the beginning of parameter parsing.
Oct 12, 2019
122748a
Added changelog file.
Oct 12, 2019
63e3d89
Fixed linting.
Oct 12, 2019
122cf60
Always creating list for consistency.
Sup3rGeo Oct 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/5946.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed issue when parametrizing fixtures with numpy arrays (and possibly other sequence-like types).
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def main():
"mock",
"nose",
"requests",
"numpy",
Copy link
Contributor

Choose a reason for hiding this comment

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

@nicoddemus
Adding numpy to all builds make the numpy tox env obsolete, doesn't it?
I'd rather not have it installed always, but in case this is kept it should clean up the env, jobs, and conditional skips we have for it.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, thanks for catching that.

Practically I think it makes sense to install an additional dependency in order to have less jobs, as an extra dependency is usually much quicker to install than starting up an entire new job, which potentionally might run all tests again.

I will follow up with a fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just started with it already myself.

Copy link
Contributor

@blueyed blueyed Oct 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I wanted to keep numpy optional, but what you've said sounds different?
Keep in mind that numpy might be very slow to install without a wheel.

=> #5977

Copy link
Member

Choose a reason for hiding this comment

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

You are right.

Let's remove numpy from extras and use importorskip on the test.

I have the diff ready:

diff --git a/setup.py b/setup.py
index be38857b0..dcf63f6fd 100644
--- a/setup.py
+++ b/setup.py
@@ -28,7 +28,6 @@ def main():
                 "mock",
                 "nose",
                 "requests",
-                "numpy",
                 "xmlschema",
             ]
         },
diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py
index 4fcab0245..9ef7f0245 100644
--- a/testing/python/fixtures.py
+++ b/testing/python/fixtures.py
@@ -4190,6 +4190,7 @@ def test_indirect_fixture_does_not_break_scope(testdir):


 def test_fixture_parametrization_nparray(testdir):
+    pytest.importorskip("numpy")
     testdir.makepyfile(
         """
         from numpy import linspace

"xmlschema",
]
},
Expand Down
5 changes: 3 additions & 2 deletions src/_pytest/fixtures.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,9 @@ def fixture(
``fixture_<fixturename>`` and then use
``@pytest.fixture(name='<fixturename>')``.
"""
if params is not None:
params = list(params)

fixture_function, arguments = _parse_fixture_args(
callable_or_scope,
*args,
Expand All @@ -1134,8 +1137,6 @@ def fixture(
fixture_function
)

if params is not None and not isinstance(params, (list, tuple)):
params = list(params)
return FixtureFunctionMarker(scope, params, autouse, ids=ids, name=name)


Expand Down
18 changes: 18 additions & 0 deletions testing/python/fixtures.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4187,3 +4187,21 @@ def test_check_fixture_instantiations():
)
result = testdir.runpytest()
result.assert_outcomes(passed=7)


def test_fixture_parametrization_nparray(testdir):
testdir.makepyfile(
"""
from numpy import linspace
from pytest import fixture

@fixture(params=linspace(1, 10, 10))
def value(request):
return request.param

def test_bug(value):
assert value == value
"""
)
result = testdir.runpytest()
result.assert_outcomes(passed=10)