Skip to content

assert_all_close doesn't exist, make it assert_array_equal#3391

Merged
emmanuelle merged 1 commit intoscikit-image:masterfrom
hmaarrfk:np_assert_all_close
Sep 7, 2018
Merged

assert_all_close doesn't exist, make it assert_array_equal#3391
emmanuelle merged 1 commit intoscikit-image:masterfrom
hmaarrfk:np_assert_all_close

Conversation

@hmaarrfk
Copy link
Copy Markdown
Member

@hmaarrfk hmaarrfk commented Sep 6, 2018

assert_allclose exists, but I think in this case we should make a
stronger assertion

@jni

Re: PR #3052

For reviewers

(Don't remove the checklist below.)

  • 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

assert_allclose exists, but I think in this case we should make a 
stronger assertion
@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 6, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@2f436f1). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3391   +/-   ##
=========================================
  Coverage          ?   86.81%           
=========================================
  Files             ?      341           
  Lines             ?    27406           
  Branches          ?        0           
=========================================
  Hits              ?    23792           
  Misses            ?     3614           
  Partials          ?        0
Impacted Files Coverage Δ
skimage/util/tests/test_dtype.py 100% <100%> (ø)

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 2f436f1...77077b6. Read the comment docs.

Copy link
Copy Markdown
Member

@jni jni left a comment

Choose a reason for hiding this comment

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

Holy moly. Thanks @hmaarrfk! I agree that it can be stronger here.

@emmanuelle
Copy link
Copy Markdown
Member

LGTM, merging (and I think it will fix Travis which is broken because of this test).

@emmanuelle emmanuelle merged commit 7549015 into scikit-image:master Sep 7, 2018
@hmaarrfk

This comment has been minimized.

@lumberbot-app

This comment has been minimized.

@lumberbot-app lumberbot-app bot added the Still Needs Manual Backport MrMeeseeks-managed label label Sep 7, 2018
@hmaarrfk

This comment has been minimized.

@lumberbot-app

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Still Needs Manual Backport MrMeeseeks-managed label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants