[MNT] Fix warnings in widget tests by janezd · Pull Request #3502 · biolab/orange3

janezd · 2018-12-24T00:12:41Z

Issue

Logs are polluted with warnings. Some are relevant and are fixed, and some are expected by tests and should be silenced. Logs are also printed out instead of being tested.

Description of changes

Includes

Code changes

codecov · 2018-12-25T13:43:47Z

Codecov Report

Merging #3502 into master will increase coverage by <.01%.
The diff coverage is 95.55%.

@@            Coverage Diff             @@
##           master    #3502      +/-   ##
==========================================
+ Coverage   83.67%   83.68%   +<.01%     
==========================================
  Files         370      370              
  Lines       66176    66293     +117     
==========================================
+ Hits        55372    55476     +104     
- Misses      10804    10817      +13

codecov · 2018-12-25T13:43:47Z

Codecov Report

Merging #3502 into master will decrease coverage by 0.04%.
The diff coverage is 99.1%.

@@            Coverage Diff             @@
##           master    #3502      +/-   ##
==========================================
- Coverage   83.59%   83.55%   -0.05%     
==========================================
  Files         367      367              
  Lines       65570    65751     +181     
==========================================
+ Hits        54814    54935     +121     
- Misses      10756    10816      +60

Orange/widgets/data/owfile.py

Orange/widgets/data/tests/test_owdatasampler.py

Orange/statistics/util.py

Orange/widgets/data/owrank.py

Orange/classification/sgd.py

janezd · 2018-12-25T21:38:23Z

Orange/widgets/report/tests/test_report.py

    def test_report_widgets_evaluate(self):
        rep = OWReport.get_instance()
-        data = Table("zoo")
+        data = Table("titanic")


This is changed to avoid warning about <4 instances in each fold of CV.

zoo has a class value that appears only 4 times. If CV is run with k=3, everything should be fine.
Running CV with less folds would probably be good practice even when not needed, since it is just as good in almost all tests and 3x faster.
Changing to titanic instead is probably quite a slowdown since it has 20x as many instances.
(not sure this makes any difference here)

Orange/widgets/unsupervised/owcorrespondence.py

Orange/widgets/data/owrank.py

Orange/widgets/tests/test_widget.py

Orange/widgets/data/tests/test_owdatasampler.py

Orange/statistics/util.py

Orange/widgets/data/owrank.py

lanzagar · 2019-01-11T14:36:57Z

Orange/widgets/report/tests/test_report.py

    def test_report_widgets_evaluate(self):
        rep = OWReport.get_instance()
-        data = Table("zoo")
+        data = Table("titanic")


zoo has a class value that appears only 4 times. If CV is run with k=3, everything should be fine.
Running CV with less folds would probably be good practice even when not needed, since it is just as good in almost all tests and 3x faster.
Changing to titanic instead is probably quite a slowdown since it has 20x as many instances.
(not sure this makes any difference here)

Orange/projection/manifold.py

lanzagar · 2019-01-15T13:48:45Z

Orange/widgets/visualize/owscatterplot.py

+            is_enabled = self.data is not None and \
+                not self.data.is_sparse() and \
+                len(self.xy_model) > 2 and len(self.data[self.valid_data]) > 1 \
+                and np.all(np.nan_to_num(np.nanstd(self.data.X, 0)) != 0)


At this point, I am starting to dislike filterwarnings more and more.
It may be the simplest solution and acceptable in some tests, but in actual code I think in most cases it points to some problems that we should either fix or explicitly check for and not trigger warnings. Filtering them out just makes it too easy to end up with bad code and make it harder to notice problems.

This line specifically is really interesting to take a closer look at.
Why would we trigger warnings in nanstd when the data is all-nan, just to change those nans to zeros and check that there are no zeros (together with checking that no std is zero). Instead of adding 4 lines to filter these warnings (in the whole function), we can add 1 line with an explicit check that no column should be all-nan before computing std and make it easier to read too.

And then looking at that line, we ask ourselves - why should a single feature with std=0 or all-nan prevent vizrank to be used at all? It would still be useful to score other, normal features.
Also, the check tests the whole X, which is now wrong for 2 reasons: meta features have been added to the list and should be treated as other features; categorical vars are no longer used in scatter plot and should be excluded from this check

Conclusion: I think we should remove this change and fix this in a separate PR

You're right, I removed this from the commit.

Orange/widgets/visualize/owscatterplotgraph.py

Orange/widgets/unsupervised/owtsne.py

Orange/widgets/visualize/owboxplot.py

lanzagar · 2019-01-15T14:19:00Z

Orange/widgets/visualize/owsieve.py

-            (self.observed - self.expected) / np.sqrt(self.expected)
+        with np.errstate(divide="ignore", invalid="ignore"):
+            self.residuals = \
+                (self.observed - self.expected) / np.sqrt(self.expected)


Can we not divide by zero instead of ignoring warnings?

No, it's a vector. I could create a mask, but it would be annoying. It's simpler this way.

Orange/classification/naive_bayes.py

…t; replace zoo with titanic to ensure enough instances in each class

…for hiearachical clustering

…eural networks and sgd

…lor data is nan

… in the widget

…d later

…solete

… parameters

lanzagar · 2019-01-18T08:45:38Z

Orange/statistics/util.py

 def _nan_min_max(x, func, axis=0):
    if not sp.issparse(x):
-        return func(x, axis=axis)
+        with warnings.catch_warnings():


This currently still changes the behaviour of nanmin/nanmax compared to numpy's version.
I thought we agreed that for numpy arrays our functions should be equivalent to np.nanmin/max

janezd changed the title ~~[MNT] Fix warnings in widget tests~~ [WIP] [MNT] Fix warnings in widget tests Dec 24, 2018

janezd force-pushed the fix-test-warnings branch 4 times, most recently from d14f153 to d52122c Compare December 25, 2018 13:43

janezd force-pushed the fix-test-warnings branch 4 times, most recently from 515bdc6 to 399bc33 Compare December 25, 2018 22:11

janezd changed the title ~~[WIP] [MNT] Fix warnings in widget tests~~ [MNT] Fix warnings in widget tests Dec 25, 2018

janezd force-pushed the fix-test-warnings branch 6 times, most recently from 612c6a8 to c5de535 Compare December 26, 2018 13:25

janezd commented Dec 26, 2018

View reviewed changes

janezd mentioned this pull request Dec 26, 2018

[MNT] Fix test warnings in scripting part #3503

Merged

janezd force-pushed the fix-test-warnings branch from c5de535 to 9c4b96c Compare January 4, 2019 12:32

janezd assigned lanzagar Jan 11, 2019

lanzagar reviewed Jan 11, 2019

View reviewed changes

Orange/widgets/data/owrank.py Outdated Show resolved Hide resolved

Orange/widgets/data/owrank.py Outdated Show resolved Hide resolved

janezd force-pushed the fix-test-warnings branch from c85d987 to 26d3022 Compare January 11, 2019 15:20

lanzagar reviewed Jan 11, 2019

View reviewed changes

janezd force-pushed the fix-test-warnings branch from 19a4816 to 583e87a Compare January 11, 2019 20:44

lanzagar reviewed Jan 15, 2019

View reviewed changes

Orange/classification/naive_bayes.py Outdated Show resolved Hide resolved

Orange/classification/naive_bayes.py Outdated Show resolved Hide resolved

Test warnings: Fix warning about old-style widget messages

88e49ae

janezd and others added 22 commits January 17, 2019 19:41

Test warnings: Test logged error in report test instead of printing i…

919468f

…t; replace zoo with titanic to ensure enough instances in each class

Test warnings: Ignore division by zero in owcorrespondence

16cb108

Test warnings: Silence warning in construction of infinite distances …

a171cb2

…for hiearachical clustering

MDS: Don't set n_init to 4 if init_data is given

f1ad559

Test warnings: Silence matrix deprecation warning in scipy

6b4e0cd

Test warnings: Silence convergence warnings in tests for widgets of n…

93f135d

…eural networks and sgd

Test warnings: Fix or silence warning in MDS tests

1a1234f

Scatterplotgraph: Faster construction of continuous palette if all co…

2adcf6c

…lor data is nan

OWManifoldLearning: capture and show warning about disconnected graph…

e8aeee6

… in the widget

OWtSNE: treat all-nan columns as constant in check_data

b92a126

Test warnings: Silence division by zero warning in PCA

c318db6

Box plot: Fix divisions by zero in statistical tests

ef82ba3

OWDistributions: Fix arguments in smoothing (float -> int)

5af6da7

Orange.distance: silence warning about unhandled nans that are handle…

d64d773

…d later

OWSieve: Fix division by zero

598bbb8

Heatmap tests: Silence warnings about convergence

28799b8

Test concurrent: Silence warning that the class (we're testing) is ob…

f9b1edc

…solete

Test warnings: Remove prints

03055fb

Tests: Stop report window and some other widgets from showing

bd289f6

owfeaturestatistics: Change deprecated locale.format -> format_string

7dd696f

Fix warnings in widget tests: Follow suggestions in review (1)

7884e2c

data.utils.histogram: Fix warning for means of empty bins

8950666

janezd force-pushed the fix-test-warnings branch from 583e87a to 5f5847a Compare January 17, 2019 18:41

janezd mentioned this pull request Jan 17, 2019

[ENH] Naive Bayes: Implement predict, fix predict_storage #3540

Merged

2 tasks

janezd added 2 commits January 17, 2019 21:23

Venn Diagram: Silence warning about nans

3395a0c

manifold.MDS: Change the signature back to same as skl, fix n_init in…

23d674d

… parameters

janezd force-pushed the fix-test-warnings branch from 5f5847a to 23d674d Compare January 17, 2019 20:23

lanzagar reviewed Jan 18, 2019

View reviewed changes

fixup! Test warnings: Remove warnings in OWRank

c5f8118

lanzagar merged commit 0637c96 into biolab:master Jan 18, 2019

Uh oh!

Conversation

janezd commented Dec 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description of changes

Includes

Uh oh!

codecov bot commented Dec 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov bot commented Dec 25, 2018

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

janezd commented Dec 24, 2018 •

edited

Loading

codecov bot commented Dec 25, 2018 •

edited

Loading