TEST: Suppress expected warnings #949

effigies · 2020-08-22T18:06:16Z

Tests are currently generating a huge number of warnings, which makes outputs less informative than they could be.

Will add subdirectories soon, but want to get CI going to make sure I'm not breaking things.

codecov · 2020-08-22T18:08:07Z

Codecov Report

Merging #949 into master will increase coverage by 0.02%.
The diff coverage is 97.05%.

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
+ Coverage   91.84%   91.87%   +0.02%     
==========================================
  Files          97       98       +1     
  Lines       12428    12451      +23     
  Branches     2191     2193       +2     
==========================================
+ Hits        11415    11439      +24     
  Misses        678      678              
+ Partials      335      334       -1

Impacted Files	Coverage Δ
nibabel/cifti2/cifti2.py	`96.69% <ø> (ø)`
nibabel/testing/__init__.py	`99.05% <85.71%> (-0.95%)`	⬇️
nibabel/casting.py	`86.32% <100.00%> (+0.17%)`	⬆️
nibabel/conftest.py	`100.00% <100.00%> (ø)`
nibabel/deprecator.py	`100.00% <100.00%> (ø)`
nibabel/keywordonly.py	`100.00% <100.00%> (ø)`
nibabel/loadsave.py	`92.59% <100.00%> (ø)`
nibabel/nicom/ascconv.py	`89.87% <100.00%> (ø)`
nibabel/nicom/dicomreaders.py	`58.76% <100.00%> (ø)`
nibabel/processing.py	`100.00% <100.00%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e65b865...1dc4e8d. Read the comment docs.

effigies · 2020-08-23T03:26:14Z

Down to a much more reasonable warnings list:

=============================== warnings summary ===============================
/home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/deprecated.py:35
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/deprecated.py:35: DeprecationWarning: The trackvis interface has been deprecated and will be removed in v4.0; please use the 'nibabel.streamlines' interface.
    mod = __import__(self._module_name, fromlist=[''])
/home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/py/_path/local.py:704
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/py/_path/local.py:704: DeprecationWarning: We will remove this module from nibabel 5.0. Please use the built-in Python `*` argument to ensure keyword-only parameters (see PEP 3102).
    __import__(modname)
/home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/py/_path/local.py:704
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/py/_path/local.py:704: FutureWarning: We no longer carry a copy of the 'py3k' module in nibabel; Please import from the 'numpy.compat.py3k' module directly. Full removal scheduled for nibabel 4.0.
    __import__(modname)
/home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/py/_path/local.py:704
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/py/_path/local.py:704: UserWarning: The DICOM readers are highly experimental, unstable, and only work for Siemens time-series at the moment
  Please use with caution.  We would be grateful for your help in improving them
    __import__(modname)
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/externals/tests/test_netcdf.py::test_itemset_no_segfault_on_readonly
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_array_interface_with_dtype
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_asarray
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_fileobj_isolated
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_header_isolated
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_is_proxy
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_ndim
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_proxy_slicing
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_proxy_api.py::TestMinc1API::test_shape
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/externals/netcdf.py:326: RuntimeWarning: Cannot close a netcdf_file opened with mmap=True, when netcdf_variables or arrays referring to its data still exist. All data arrays obtained from such files refer directly to data on disk, and must be copied before the file can be cleanly closed. (See netcdf_file docstring for more information on mmap.)
    ), category=RuntimeWarning)
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/freesurfer/tests/test_mghformat.py: 12 warnings
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_analyze.py: 17 warnings
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_minc1.py: 12 warnings
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_nifti1.py: 35 warnings
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_nifti2.py: 68 warnings
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_spatialimages.py: 4 warnings
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_spm2analyze.py: 20 warnings
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_spm99analyze.py: 15 warnings
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_spatialimages.py:525: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray.
    np.random.choice(slice_elems, n_elems).tolist())
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/streamlines/tests/test_tractogram.py::TestTractogram::test_creating_invalid_tractogram
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/streamlines/tests/test_tractogram.py::TestTractogram::test_tractogram_creation
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/numpy/core/_asarray.py:86: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray.
    return array(a, dtype, copy=False, order=order)
venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_filehandles.py::test_multiload
  /home/travis/build/nipy/nibabel/venv/lib/python3.6/site-packages/nibabel-3.1.1+86.g2fc0b52-py3.6.egg/nibabel/tests/test_filehandles.py:29: UserWarning: It would take too long to test file handles, aborting
    warn('It would take too long to test file handles, aborting')

Filtering on-import warnings feels like more work than it's worth. Handling the NetCDF case would need its own PR. Looks like a couple warnings that actually need dealing with for upcoming Numpy...

effigies

Some notes for a potential reviewer...

nibabel/freesurfer/tests/test_io.py

nibabel/loadsave.py

effigies · 2020-08-23T03:34:06Z

nibabel/nicom/ascconv.py

@@ -205,7 +205,7 @@ def parse_ascconv(ascconv_str, str_delim='"'):
    attrs, content = ASCCONV_RE.match(ascconv_str).groups()
    attrs = OrderedDict((tuple(x.split('=')) for x in attrs.split()))
    # Normalize string start / end markers to something Python understands
-    content = content.replace(str_delim, '"""')
+    content = content.replace(str_delim, '"""').replace("\\", "\\\\")


This one seems like a reasonable fix, but it should get @matthew-brett's or @moloney's eyes to be sure. Previously the line:

tSequenceFileName = ""%SiemensSeq%\ep2d_diff""

Resulted in ast.parse() emitting:

nibabel/nicom/tests/test_ascconv.py::test_ascconv_parse <unknown>:2: DeprecationWarning: invalid escape sequence \e

nibabel/processing.py

nibabel/streamlines/tests/test_streamlines.py

nibabel/tests/test_image_api.py

effigies · 2020-09-01T19:34:15Z

@anibalsolon @djarecka @orduek @robbisg Could I trouble one of you for a review?

djarecka · 2020-09-02T14:25:24Z

@effigies - I can take a look, but I was never working on removing the warnings, so don't have experience with it. One question I have after quickly checking pytest website - have you tried --disable-warnings? Or it's not what you want?

effigies · 2020-09-02T14:30:38Z

No, I want warnings to show if they're valid, as that's one of the better ways to see upcoming deprecations in Python or numpy. But expected warnings that we produce shouldn't clutter the outputs.

robbisg · 2020-09-02T14:33:34Z

I can help, but as @djarecka, I never worked with suppressing warnings. ;)

djarecka · 2020-09-02T15:00:37Z

@effigies - ok, so you're concerned about your own warnings and you want to test them and remove from the output at the sam time. got it!

…n warning

djarecka · 2020-09-04T04:15:45Z

nibabel/freesurfer/tests/test_mghformat.py

@@ -345,11 +345,14 @@ def test_deprecated_fields():

    # mrparams is the only deprecated field at the moment
    # Accessing hdr_data is equivalent to accessing hdr, so double all checks
-    assert_array_equal(hdr['mrparams'], 0)
+    with pytest.deprecated_call():


fyi. if you want to check specific warning, you can also use with pytest.deprecated_call(match="deprecated from version: 2.3")

djarecka · 2020-09-04T04:30:53Z

nibabel/streamlines/tests/test_streamlines.py

-            assert issubclass(w[0].category, ExtensionWarning)
-            assert "extension" in str(w[0].message)
+        assert len(w) == 1
+        assert "extension" in str(w[0].message)


you could also use with pytest.warns(ExtensionWarning, match="extension")

djarecka · 2020-09-04T04:42:26Z

@effigies -lgtm, i don't remember why we didn't move from clear_and_catch_warnings. I thought we had some reasons, but perhaps not very important ones.
I recently discovered match in pytest.raises etc., and I really like it, so showed you some examples for future.

effigies · 2020-09-05T13:25:18Z

i don't remember why we didn't move from clear_and_catch_warnings.

I don't think I realized that pytest was breaking its behavior, and it seemed like an extra task to insert after getting tests working.

Thanks for the suggestions. I think I'll go and look for all the warnings and improve them...

effigies · 2020-09-06T19:40:46Z

Updated. Would appreciate another look through when you get a chance.

nibabel/streamlines/tests/test_tck.py

Co-authored-by: Dorota Jarecka <djarecka@gmail.com>

effigies · 2020-09-10T12:44:52Z

All green. Good to merge?

djarecka · 2020-09-10T22:13:22Z

@effigies - I've just opened diff quickly, and wonder if there is any specific reason why you use unittest for skipping tests (e.g.unittest.skipIf) instead of pytest.mark.skipif ?
haven't noticed this before

effigies · 2020-09-10T22:25:39Z

Yes. When there's a standard library equivalent, I'm preferring that, on the grounds that one day we may need to migrate from pytest as we did from nose. The more things that are vanilla Python, the less work that should be.

djarecka · 2020-09-10T22:41:14Z

ok, i'm still hoping that we don't have to move from pytest anytime soon :-)
lgtm

effigies · 2020-09-10T22:51:28Z

Nose lasted 10 years, so if pytest does as well, we still have plenty of time.

effigies · 2020-09-10T22:51:52Z

Thanks for reviewing!

effigies commented Aug 23, 2020

View reviewed changes

effigies added 14 commits September 2, 2020 09:25

TEST: Suppress expected warnings

f0ab879

FIX: Avoid/suppress expected warnings

2f461be

FIX: Bad deprecate_with_version calls

d4f2216

PY36: Backport nullcontext

f879473

DOCTEST: Suppress deprecation warnings in doctests of deprecated methods

8903f0b

TEST: Suppress warnings in freesurfer

c7fad2e

MNT: Add pytest-doctestplus to ignore warnings in doctests

7acde5a

TEST: Suppress and check warnings in streamlines

72fa2d2

TEST: Use get_fdata, check for expected warning

470995d

RF: Drop calls to deprecated get_affine()

dc36a28

FIX: Escape backslashes before parsing strings

86b8f6c

MNT: Reset deprecation clock

3694a9a

FIX: Remove doubled-up fix for divide-by-zero warning

963aee4

TEST: Small cleanups

786cf99

effigies force-pushed the test/reduce_warnings branch from 3ca4ece to 786cf99 Compare September 2, 2020 13:28

effigies added 3 commits September 2, 2020 12:05

FIX: Construct sliceobj from object array; addresses numpy deprecatio…

0634da4

…n warning

TEST: Use skipIf rather than warn + return for test_multiload

27146c6

TEST: Check import-time warnings on modules via conftest

dae2f86

djarecka reviewed Sep 4, 2020

View reviewed changes

effigies added 3 commits September 6, 2020 15:32

TEST: Use matches or comments to make clear expected warnings

36a1adf

TEST: Unused imports

4707222

TEST: Simplify lazy_load conditional warning check

f1bfc39

djarecka reviewed Sep 10, 2020

View reviewed changes

nibabel/streamlines/tests/test_tck.py Outdated Show resolved Hide resolved

Update nibabel/streamlines/tests/test_tck.py

1dc4e8d

Co-authored-by: Dorota Jarecka <djarecka@gmail.com>

effigies merged commit acd69ea into nipy:master Sep 10, 2020

effigies deleted the test/reduce_warnings branch October 16, 2020 14:56

TEST: Suppress expected warnings #949

TEST: Suppress expected warnings #949

Uh oh!

Conversation

effigies commented Aug 22, 2020

Uh oh!

codecov bot commented Aug 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

effigies commented Aug 23, 2020

Uh oh!

effigies left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

effigies Aug 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

effigies commented Sep 1, 2020

Uh oh!

djarecka commented Sep 2, 2020

Uh oh!

effigies commented Sep 2, 2020

Uh oh!

robbisg commented Sep 2, 2020

Uh oh!

djarecka commented Sep 2, 2020

Uh oh!

djarecka Sep 4, 2020

Choose a reason for hiding this comment

Uh oh!

djarecka Sep 4, 2020

Choose a reason for hiding this comment

Uh oh!

djarecka commented Sep 4, 2020

Uh oh!

effigies commented Sep 5, 2020

Uh oh!

effigies commented Sep 6, 2020

Uh oh!

Uh oh!

effigies commented Sep 10, 2020

Uh oh!

djarecka commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

effigies commented Sep 10, 2020

Uh oh!

djarecka commented Sep 10, 2020

Uh oh!

effigies commented Sep 10, 2020

Uh oh!

effigies commented Sep 10, 2020

Uh oh!

Uh oh!

codecov bot commented Aug 22, 2020 •

edited

Loading

djarecka commented Sep 10, 2020 •

edited

Loading