-
Notifications
You must be signed in to change notification settings - Fork 587
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
Avoid being the first to import large packages #2204
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,8 @@ | ||
RELEASE_TYPE: patch | ||
|
||
This patch avoids importing test runners such as :pypi`pytest`, :pypi`unittest2`, | ||
or :pypi`nose` solely to access their special "skip test" exception types - | ||
if the module is not in ``sys.modules``, the exception can't be raised anyway. | ||
|
||
This fixes a problem where importing an otherwise unused module could cause | ||
spurious errors due to import-time side effects (and possibly ``-Werror``). |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,14 +26,6 @@ | |
struct_unpack, | ||
) | ||
|
||
try: | ||
import numpy | ||
except (ImportError, TypeError): # pragma: no cover | ||
# We catch TypeError because that can be raised if Numpy is installed on | ||
# PyPy for Python 2.7; and we only need a workaround until 2020-01-01. | ||
numpy = None | ||
|
||
|
||
# Format codes for (int, float) sized types, used for byte-wise casts. | ||
# See https://docs.python.org/3/library/struct.html#format-characters | ||
STRUCT_FORMATS = { | ||
|
@@ -46,25 +38,30 @@ | |
# There are two versions of this: the one that uses Numpy to support Python | ||
# 3.5 and earlier, and the elegant one for new versions. We use the new | ||
# one if Numpy is unavailable too, because it's slightly faster in all cases. | ||
if numpy and not CAN_PACK_HALF_FLOAT: # pragma: no cover | ||
|
||
def reinterpret_bits(x, from_, to): | ||
if from_ == b"!e": | ||
arr = numpy.array([x], dtype=">f2") | ||
if numpy.isfinite(x) and not numpy.isfinite(arr[0]): | ||
quiet_raise(OverflowError("%r too large for float16" % (x,))) | ||
buf = arr.tobytes() | ||
else: | ||
buf = struct_pack(from_, x) | ||
if to == b"!e": | ||
return float(numpy.frombuffer(buf, dtype=">f2")[0]) | ||
return struct_unpack(to, buf)[0] | ||
def reinterpret_bits(x, from_, to): | ||
return struct_unpack(to, struct_pack(from_, x))[0] | ||
|
||
|
||
else: | ||
if not CAN_PACK_HALF_FLOAT: # pragma: no cover | ||
try: | ||
import numpy | ||
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. Do we not care about non-lazily importing numpy in this case? I guess it only matters until January. 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. Also for Python 3.5, so until September! But basically no, I don't care about this case - pypistats show very few people still using 3.5. A lazy import here would be pretty awful code, impact the floats() check too, and we'd be taking it out in 10 months anyway. I'll add it if someone asks, though. |
||
except (ImportError, TypeError): | ||
# We catch TypeError because that can be raised if Numpy is installed on | ||
# PyPy for Python 2.7; and we only need a workaround until 2020-01-01. | ||
pass | ||
else: | ||
|
||
def reinterpret_bits(x, from_, to): | ||
return struct_unpack(to, struct_pack(from_, x))[0] | ||
def reinterpret_bits(x, from_, to): # pylint: disable=function-redefined | ||
if from_ == b"!e": | ||
arr = numpy.array([x], dtype=">f2") | ||
if numpy.isfinite(x) and not numpy.isfinite(arr[0]): | ||
quiet_raise(OverflowError("%r too large for float16" % (x,))) | ||
buf = arr.tobytes() | ||
else: | ||
buf = struct_pack(from_, x) | ||
if to == b"!e": | ||
return float(numpy.frombuffer(buf, dtype=">f2")[0]) | ||
return struct_unpack(to, buf)[0] | ||
|
||
|
||
def float_of(x, width): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# coding=utf-8 | ||
# | ||
# This file is part of Hypothesis, which may be found at | ||
# https://github.com/HypothesisWorks/hypothesis/ | ||
# | ||
# Most of this work is copyright (C) 2013-2019 David R. MacIver | ||
# ([email protected]), but it contains contributions by others. See | ||
# CONTRIBUTING.rst for a full list of people who may hold copyright, and | ||
# consult the git log if you need to determine who owns an individual | ||
# contribution. | ||
# | ||
# This Source Code Form is subject to the terms of the Mozilla Public License, | ||
# v. 2.0. If a copy of the MPL was not distributed with this file, You can | ||
# obtain one at https://mozilla.org/MPL/2.0/. | ||
# | ||
# END HEADER | ||
|
||
from __future__ import absolute_import, division, print_function | ||
|
||
import subprocess | ||
import sys | ||
|
||
SHOULD_NOT_IMPORT_TEST_RUNNERS = """ | ||
import sys | ||
import unittest | ||
from hypothesis import given, strategies as st | ||
|
||
class TestDoesNotImportRunners(unittest.TestCase): | ||
strat = st.integers() | st.floats() | st.sampled_from(["a", "b"]) | ||
|
||
@given(strat) | ||
def test_does_not_import_unittest2(self, x): | ||
assert "unittest2" not in sys.modules | ||
|
||
@given(strat) | ||
def test_does_not_import_nose(self, x): | ||
assert "nose" not in sys.modules | ||
|
||
@given(strat) | ||
def test_does_not_import_pytest(self, x): | ||
assert "pytest" not in sys.modules | ||
|
||
if __name__ == '__main__': | ||
unittest.main() | ||
""" | ||
|
||
|
||
def test_hypothesis_does_not_import_test_runners(tmp_path): | ||
# We obviously can't use pytest to check that pytest is not imported, | ||
# so for consistency we use unittest for all three non-stdlib test runners. | ||
# It's unclear which of our dependencies is importing unittest, but | ||
# since I doubt it's causing any spurious failures I don't really care. | ||
# See https://github.com/HypothesisWorks/hypothesis/pull/2204 | ||
fname = str(tmp_path / "test.py") | ||
with open(fname, "w") as f: | ||
f.write(SHOULD_NOT_IMPORT_TEST_RUNNERS) | ||
subprocess.check_call([sys.executable, fname]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
# coding=utf-8 | ||
# | ||
# This file is part of Hypothesis, which may be found at | ||
# https://github.com/HypothesisWorks/hypothesis/ | ||
# | ||
# Most of this work is copyright (C) 2013-2019 David R. MacIver | ||
# ([email protected]), but it contains contributions by others. See | ||
# CONTRIBUTING.rst for a full list of people who may hold copyright, and | ||
# consult the git log if you need to determine who owns an individual | ||
# contribution. | ||
# | ||
# This Source Code Form is subject to the terms of the Mozilla Public License, | ||
# v. 2.0. If a copy of the MPL was not distributed with this file, You can | ||
# obtain one at https://mozilla.org/MPL/2.0/. | ||
# | ||
# END HEADER | ||
|
||
from __future__ import absolute_import, division, print_function | ||
|
||
from hypothesis.internal.compat import CAN_PACK_HALF_FLOAT | ||
|
||
SHOULD_NOT_IMPORT_NUMPY = """ | ||
import sys | ||
from hypothesis import given, strategies as st | ||
|
||
@given(st.integers() | st.floats() | st.sampled_from(["a", "b"])) | ||
def test_no_numpy_import(x): | ||
assert "numpy" not in sys.modules | ||
Zac-HD marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
|
||
|
||
def test_hypothesis_is_not_the_first_to_import_numpy(testdir): | ||
result = testdir.runpytest(testdir.makepyfile(SHOULD_NOT_IMPORT_NUMPY)) | ||
# OK, we import numpy on Python < 3.6 to get 16-bit float support. | ||
# But otherwise we only import it if the user did so first. | ||
if CAN_PACK_HALF_FLOAT: | ||
result.assert_outcomes(passed=1, failed=0) | ||
else: | ||
result.assert_outcomes(passed=0, failed=1) |
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.
I am not wild about running these methods on every exception raised - given that that raising exceptions is the main thing Hypothesis tests do it's pretty hot path, and doing all this conditional querying as to whether stuff is already in
sys.modules
is technically fine but looks pretty weird and results in all of these coverage pragmas.How about instead catching all exceptions and checking whether the current exception is one to reraise based on a string version of its name? e.g. something like:
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.
That really doesn't look better to me! The sys.modules checks are unusual, but the whole function is fast - we have four dict lookups, and up to five attribute accesses. The slowest part is sorting the types by str!
And we already had those coverage pragmas, spread out across more lines of code.