Skip to content

RF+BF: Add tolerances and data types for nib-diff, remove hypothesis dependency #678

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

Merged
merged 24 commits into from
Oct 11, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
cd5a741
RF+ENH: nib-diff - allow to specify absolute and/or relative maximal …
yarikoptic Sep 14, 2018
018eceb
ENH: nib-diff Field/File not just Field in the header
yarikoptic Sep 14, 2018
833b4df
changed as commented out in the pull request
chrispycheng Sep 21, 2018
9b123f6
changes as commented
chrispycheng Sep 21, 2018
1e33ea7
RF: anticipated files of different shapes, fixed table display, corre…
chrispycheng Sep 25, 2018
76ca32f
elaborated docstring, modified get_data_diff to allow direct array in…
chrispycheng Sep 27, 2018
0aa6370
added to diff documentation, undid executable change, took out debugg…
chrispycheng Sep 28, 2018
d057249
undid permission snafu on test_scripts
chrispycheng Sep 28, 2018
76ee358
docstring and function name clarification, change get_data to get_fda…
chrispycheng Oct 2, 2018
034c276
corrected styles per Travis, limited fdata to float32
chrispycheng Oct 2, 2018
19fcdd5
STY: Break overly-long line
effigies Oct 3, 2018
93c7bb6
prepared for future PR to allow modification of dtype used in diff co…
chrispycheng Oct 3, 2018
cd85e09
Merge branch 'enh-diff' of https://github.com/yarikoptic/nibabel into…
chrispycheng Oct 3, 2018
97ead00
added cmdline functionality for modifying datatype used for file data…
chrispycheng Oct 3, 2018
2340a92
added testing for dtype at cmdline, assuming cmdline functionality. c…
chrispycheng Oct 4, 2018
e8d3a4f
RF: do not use hypothesis
yarikoptic Oct 6, 2018
09cfef1
BF: properly compare NaNs and test for >2 arguments given to are_valu…
yarikoptic Oct 6, 2018
1996ead
RF: long -> large, NaN -> nan
yarikoptic Oct 8, 2018
0fb8920
BF: reintroduce try/except guarding around initial np.isnan
yarikoptic Oct 8, 2018
e4c5b52
BF: exception string matching for not implemented in numpy 1.7.1
yarikoptic Oct 8, 2018
e87d581
BF: another workaround for elderly numpy 1.7.1
yarikoptic Oct 8, 2018
f973ce4
BF(DOC): fixed damn trailing whitespaces in a docsting
yarikoptic Oct 9, 2018
22f89e9
Merge branch 'no-hypothesis' of https://github.com/yarikoptic/nibabel…
chrispycheng Oct 10, 2018
3636c4d
added tests to fix coverage problem
chrispycheng Oct 11, 2018
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
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ cache:
- $HOME/.cache/pip
env:
global:
- DEPENDS="six numpy scipy matplotlib h5py pillow pydicom hypothesis"
- DEPENDS="six numpy scipy matplotlib h5py pillow pydicom"
- OPTIONAL_DEPENDS=""
- INSTALL_TYPE="setup"
- EXTRA_WHEELS="https://5cf40426d9f06eb7461d-6fe47d9331aba7cd62fc36c7196769e4.ssl.cf2.rackcdn.com"
Expand Down Expand Up @@ -97,7 +97,7 @@ before_install:
- source venv/bin/activate
- python --version # just to check
- pip install -U pip wheel # needed at one point
- retry pip install nose flake8 mock hypothesis # always
- retry pip install nose flake8 mock # always
- pip install $EXTRA_PIP_FLAGS $DEPENDS $OPTIONAL_DEPENDS
- if [ "${COVERAGE}" == "1" ]; then
pip install coverage;
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ install:
- SET PATH=%PYTHON%;%PYTHON%\Scripts;%PATH%

# Install the dependencies of the project.
- pip install numpy scipy matplotlib nose h5py mock hypothesis pydicom
- pip install numpy scipy matplotlib nose h5py mock pydicom
- pip install .
- SET NIBABEL_DATA_DIR=%CD%\nibabel-data

Expand Down
1 change: 0 additions & 1 deletion dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@
-r requirements.txt
nose
mock
hypothesis
42 changes: 29 additions & 13 deletions nibabel/cmdline/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,41 @@ def get_opt_parser():


def are_values_different(*values):
"""Generically compares values, returns true if different"""
value0 = values[0]
values = values[1:] # to ensure that the first value isn't compared with itself
"""Generically compare values, return True if different

for value in values:
try: # we sometimes don't want NaN values
if np.any(np.isnan(value0)) and np.any(np.isnan(value)): # if they're both NaN
break
elif np.any(np.isnan(value0)) or np.any(np.isnan(value)): # if only 1 is NaN
return True
Note that comparison is targetting reporting of comparison of the headers
so has following specifics:
- even a difference in data types is considered a difference, i.e. 1 != 1.0
- NaNs are considered to be the "same", although generally NaN != NaN
"""
value0 = values[0]

except TypeError:
pass
# to not recompute over again
if isinstance(value0, np.ndarray):
value0_nans = np.isnan(value0)
if not np.any(value0_nans):
value0_nans = None

for value in values[1:]:
if type(value0) != type(value): # if types are different, then we consider them different
return True
elif isinstance(value0, np.ndarray):
return np.any(value0 != value)

if value0.dtype != value.dtype or \
value0.shape != value.shape:
return True
# there might be NaNs and they need special treatment
if value0_nans is not None:
value_nans = np.isnan(value)
if np.any(value0_nans != value_nans):
return True
if np.any(value0[np.logical_not(value0_nans)]
!= value[np.logical_not(value0_nans)]):
return True
elif np.any(value0 != value):
return True
elif value0 is np.NaN:
Copy link
Member

Choose a reason for hiding this comment

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

Numpy prefers you to use np.nan. I don't know if this means they plan to remove the synonyms in the future or if it's just a style thing. I'd go with their recommendation, though.

Also appears in the test file.

if value is not np.NaN:
return True
elif value0 != value:
return True

Expand Down
97 changes: 51 additions & 46 deletions nibabel/tests/test_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
from os.path import (dirname, join as pjoin, abspath)
import numpy as np

from hypothesis import given
import hypothesis.strategies as st


DATA_PATH = abspath(pjoin(dirname(__file__), 'data'))

Expand All @@ -18,51 +15,59 @@
# TODO: MAJOR TO DO IS TO FIGURE OUT HOW TO USE HYPOTHESIS FOR LONGER LIST LENGTHS WHILE STILL CONTROLLING FOR OUTCOMES


@given(st.data())
def test_diff_values_int(data):
x = data.draw(st.integers(), label='x')
y = data.draw(st.integers(min_value=x + 1), label='x+1')
z = data.draw(st.integers(max_value=x - 1), label='x-1')

assert not are_values_different(x, x)
assert are_values_different(x, y)
assert are_values_different(x, z)
assert are_values_different(y, z)


@given(st.data())
def test_diff_values_float(data):
x = data.draw(st.just(0), label='x')
y = data.draw(st.floats(min_value=1e8), label='y')
z = data.draw(st.floats(max_value=-1e8), label='z')
def test_diff_values_int():
long = 10**30
Copy link
Member

Choose a reason for hiding this comment

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

long is a keyword in Python 2. Maybe large?

assert not are_values_different(0, 0)
assert not are_values_different(1, 1)
assert not are_values_different(long, long)
assert are_values_different(0, 1)
assert are_values_different(1, 2)
assert are_values_different(1, long)

assert not are_values_different(x, x)
assert are_values_different(x, y)
assert are_values_different(x, z)
assert are_values_different(y, z)

def test_diff_values_float():
assert not are_values_different(0., 0.)
assert not are_values_different(0., 0., 0.) # can take more
assert not are_values_different(1.1, 1.1)
assert are_values_different(0., 1.1)
assert are_values_different(0., 0, 1.1)
assert are_values_different(1., 2.)

@given(st.data())
def test_diff_values_mixed(data):
type_float = data.draw(st.floats(), label='float')
type_int = data.draw(st.integers(), label='int')
type_none = data.draw(st.none(), label='none')

assert are_values_different(type_float, type_int)
assert are_values_different(type_float, type_none)
assert are_values_different(type_int, type_none)
def test_diff_values_mixed():
assert are_values_different(1.0, 1)
assert are_values_different(1.0, "1")
assert are_values_different(1, "1")
assert are_values_different(1, None)
assert are_values_different(np.ndarray([0]), 'hey')
assert not are_values_different(type_none, type_none)


@given(st.data())
def test_diff_values_array(data):
a = data.draw(st.lists(elements=st.integers(min_value=0), min_size=1))
b = data.draw(st.lists(elements=st.integers(max_value=-1), min_size=1))
c = data.draw(st.lists(elements=st.floats(min_value=1e8), min_size=1))
d = data.draw(st.lists(elements=st.floats(max_value=-1e8), min_size=1))
# TODO: Figure out a way to include 0 in lists (arrays)

assert are_values_different(a, b)
assert are_values_different(c, d)
assert not are_values_different(a, a)
assert not are_values_different(None, None)


def test_diff_values_array():
from numpy import NaN, array, inf
a_int = array([1, 2])
a_float = a_int.astype(float)

assert are_values_different(a_int, a_float)
assert are_values_different(a_int, a_int, a_float)
assert are_values_different(np.arange(3), np.arange(1, 4))
assert are_values_different(np.arange(3), np.arange(4))
assert are_values_different(np.arange(4), np.arange(4).reshape((2, 2)))
# no broadcasting should kick in - shape difference
assert are_values_different(array([1]), array([1, 1]))
assert not are_values_different(a_int, a_int)
assert not are_values_different(a_float, a_float)

# NaNs - we consider them "the same" for the purpose of these comparisons
assert not are_values_different(NaN, NaN)
assert not are_values_different(NaN, NaN, NaN)
assert are_values_different(NaN, NaN, 1)
assert are_values_different(1, NaN, NaN)
assert not are_values_different(array([NaN, NaN]), array([NaN, NaN]))
assert not are_values_different(array([NaN, NaN]), array([NaN, NaN]), array([NaN, NaN]))
assert not are_values_different(array([NaN, 1]), array([NaN, 1]))
assert are_values_different(array([NaN, NaN]), array([NaN, 1]))
assert are_values_different(array([0, NaN]), array([NaN, 0]))
# and some inf should not be a problem
assert not are_values_different(array([0, inf]), array([0, inf]))
assert are_values_different(array([0, inf]), array([inf, 0]))