WIP/TST: enable/mark mangled/commented-out tests #5768

jbrockmendel · 2019-05-21T14:01:04Z

Based on a read through the coverage statistics for test files.

Might be worth adding except ImportError: to the .coveragerc exclusions; then again we might want to specifically make sure to test no-matplotlib environments.

Not remotely confident that the addition of cvxopt is in the right place. Suggestions welcome.

@ChadFulton there are a bunch of un-hit cases in statespace test files separate from the ones marked here, e.g. https://coveralls.io/builds/23474023/source?filename=statsmodels%2Ftsa%2Fstatespace%2Ftests%2Ftest_simulation_smoothing.py#L441 any thoughts on how/whether these can/should be reached? (no hurry, there's plenty in this WIP to keep me occupied for a bit)

pep8speaks · 2019-05-21T14:01:36Z

Hello @jbrockmendel! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-07-12 13:22:53 UTC

josef-pkt · 2019-05-21T14:42:59Z

statsmodels/nonparametric/tests/test_kernels.py

@@ -101,22 +101,26 @@ def test_smoothconf(self):

        #assert_allclose(fitted, res_fitted, rtol=0, atol=1e-6)

-    def t_est_smoothconf_data(self):
+    @pytest.mark.smoke  # TOOD: make this an actual test?
+    def test_smoothconf_data(self):


this one might be expensive
maybe better not to run it

could also add slow marker

josef-pkt · 2019-05-21T14:50:54Z

statsmodels/sandbox/distributions/mv_normal.py

@@ -926,11 +926,10 @@ def affine_transformed(self, shift, scale_matrix):

        Returns
        -------
-        mvt : instance of MVT
-            instance of multivariate t distribution given by affine
+        mvt : instance of MVNormal


Sounds good. BTW while looking through the sandbox.distributions code I found a lot of it to be really nice (also TheilGLS stood out). I'm looking forward to seeing what else you have in mind in this area.

josef-pkt · 2019-05-21T14:55:38Z

statsmodels/discrete/tests/test_constrained.py

@@ -65,61 +65,54 @@ def test_basic(self):


    def test_basic_method(self):


don't change this,
if hasattr(self, 'res1m') should stay in both test functions

OK. Are there more forthcoming cases where the attribute doesn't exist yet?

most likely or maybe, support for constrained fitting is still missing in some models, e.g. my discrete PR to add offset and fit_constrained to Binary models.
I don't remember the details of the unit tests without going through them.

This would change the usage for subclasses, so it's not a innocent change and should stay as the original author (I) intended.

josef-pkt · 2019-05-21T14:57:25Z

looks good overall

don't change statsmodels/discrete/tests/test_constrained.py
and a few other comments

the one kde test is the only one that looks expensive to add (and will not assert anything)

bashtage · 2019-05-22T15:40:50Z

statsmodels/datasets/tests/test_utils.py

        pytest.skip('Unable to retrieve file - skipping test')
    try:
        duncan = get_rdataset("Duncan", "carData", cache=cur_dir)
-    except (HTTPError, URLError):
+    except (HTTPError, URLError):  # pragma: no cover


These are already in coveragerc

https://coveralls.io/builds/23535701/source?filename=statsmodels/datasets/tests/test_utils.py

Looks like its not in coveragerc_travis. I'll update it there. I missed the discussion that led to having multiple coveragerc files

If we could get back to just one coveragerc file, we could drop that and put it into setup.cfg

bashtage · 2019-05-22T15:42:16Z

statsmodels/discrete/tests/test_constrained.py

-                    message = ('test: ll_0 not available, llnull=%6.4F'
-                                                                % res1.llnull)
-                    warnings.warn(message)
+        res1 = (self.res1m if not hasattr(self.res1m, '_results')


You could fix the indentation. If not hasattr(): skip then no indentation needed below.

josef and I are working together pretty well in this thread, I'd like to keep that going by not pushing on this.

What's the problem with intentation?
this looks like a regular if else to me.

The function is

if cond: Lots of lines fo test code: else: skip

Better hygiene is to do

if not cond: skip lots of test code (<- 1 level less indent, and no need to read to the bottom to figure out what is going on)

This is just hygiene, so clearly not essential.

bashtage · 2019-05-22T15:44:06Z

statsmodels/graphics/tests/test_agreement.py

@@ -7,7 +7,7 @@

 try:
    import matplotlib.pyplot as plt
-except ImportError:


You should add except ImportError to coverage. This should be skipped everywhere.

jbrockmendel · 2019-05-22T17:15:52Z

Updated with suggested edits.

Outstanding questions:

is the added pip install cvxopt in the appropriate place or can a better one be suggested?

My inclination is to separate out the passing parts of this and keep a) the newly enabled and failing tests and b) the FIXME/TODO comments in a WIP PR that I'll slowly whittle down. Thoughts?

.travis.yml

codecov · 2019-05-23T01:29:04Z

Codecov Report

Merging #5768 into master will increase coverage by 0.33%.
The diff coverage is 92.59%.

@@            Coverage Diff             @@
##           master    #5768      +/-   ##
==========================================
+ Coverage    82.3%   82.64%   +0.33%     
==========================================
  Files         593      593              
  Lines       93728    93639      -89     
  Branches    10356     9605     -751     
==========================================
+ Hits        77146    77385     +239     
+ Misses      14185    13868     -317     
+ Partials     2397     2386      -11

Impacted Files	Coverage Δ
...dels/nonparametric/tests/test_kernel_regression.py	`100% <ø> (+4.08%)`	⬆️
statsmodels/tsa/tests/test_holtwinters.py	`100% <ø> (ø)`	⬆️
statsmodels/regression/tests/test_glsar_stata.py	`100% <ø> (+4.76%)`	⬆️
statsmodels/genmod/tests/test_glm.py	`99.48% <ø> (+0.41%)`	⬆️
statsmodels/tsa/tests/test_stattools.py	`98.92% <ø> (ø)`	⬆️
statsmodels/graphics/tests/test_dotplot.py	`100% <ø> (+2.63%)`	⬆️
statsmodels/duration/tests/test_survfunc.py	`100% <ø> (+2.65%)`	⬆️
statsmodels/graphics/tests/test_boxplots.py	`100% <ø> (+3.03%)`	⬆️
statsmodels/tsa/statespace/tests/test_kalman.py	`95.36% <ø> (ø)`	⬆️
statsmodels/multivariate/tests/test_factor.py	`93.44% <ø> (+2.81%)`	⬆️
... and 93 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 68d5436...3f79cab. Read the comment docs.

codecov · 2019-05-23T01:29:04Z

Codecov Report

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

@@            Coverage Diff            @@
##             master    #5768   +/-   ##
=========================================
  Coverage          ?   82.79%           
=========================================
  Files             ?      595           
  Lines             ?    93754           
  Branches          ?    10332           
=========================================
  Hits              ?    77628           
  Misses            ?    13748           
  Partials          ?     2378

Impacted Files	Coverage Δ
statsmodels/stats/tests/test_weightstats.py	`98.52% <ø> (ø)`
statsmodels/tools/tests/test_grouputils.py	`98.79% <ø> (ø)`
statsmodels/tsa/statespace/tests/test_kalman.py	`95.36% <ø> (ø)`
statsmodels/tools/tests/test_numdiff.py	`98.37% <ø> (ø)`
statsmodels/base/tests/test_data.py	`100% <ø> (ø)`
...dels/nonparametric/tests/test_kernel_regression.py	`100% <ø> (ø)`
statsmodels/tsa/tests/test_holtwinters.py	`100% <ø> (ø)`
statsmodels/regression/tests/test_glsar_stata.py	`100% <ø> (ø)`
statsmodels/tools/tests/test_decorators.py	`100% <ø> (ø)`
statsmodels/stats/libqsturng/tests/test_qsturng.py	`84% <ø> (ø)`
... and 26 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 5f086e8...6ddafc0. Read the comment docs.

coveralls · 2019-05-23T01:46:29Z

Coverage increased (+0.3%) to 85.501% when pulling 6ddafc0 on jbrockmendel:typos into 5f086e8 on statsmodels:master.

bashtage · 2019-05-23T08:20:12Z

Clopen to trigger CI

bashtage · 2019-05-24T10:54:22Z

Needs rebase

jbrockmendel · 2019-05-24T18:16:18Z

Looks like cvxopt doesn’t provide a manylinux wheel for py34. Not sure where the six version mismatch comes from. Fine to wait until after we drop py34 since this isn’t merge-ready anyway.

jbrockmendel · 2019-05-26T23:41:24Z

statsmodels/gam/smooth_basis.py

@@ -43,7 +43,7 @@ def _R_compat_quantile(x, probs):
 def _eval_bspline_basis(x, knots, degree, deriv='all', include_intercept=True):
    try:
        from scipy.interpolate import splev
-    except ImportError:  # pragma: no cover
+    except ImportError:
        raise ImportError("spline functionality requires scipy")


this seems unreachable. maybe its a specific scipy version?

bashtage · 2019-07-12T11:33:04Z

@jbrockmendel Could you rebase so we can see if this is OK now?

jbrockmendel force-pushed the typos branch from 88ab19e to d942e35 Compare May 21, 2019 14:03

josef-pkt reviewed May 21, 2019

View reviewed changes

jbrockmendel mentioned this pull request May 22, 2019

SUMM: Roundup of sandbox exposure, unused files, redundant code, ... #5773

Open

92 tasks

bashtage reviewed May 22, 2019

View reviewed changes

jbrockmendel force-pushed the typos branch from d942e35 to 5b89e22 Compare May 22, 2019 17:12

bashtage reviewed May 22, 2019

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

jbrockmendel force-pushed the typos branch 3 times, most recently from 35c30e8 to 3f79cab Compare May 23, 2019 01:29

bashtage closed this May 23, 2019

bashtage reopened this May 23, 2019

bashtage added the type-cleanup label May 24, 2019

jbrockmendel force-pushed the typos branch from 8c22c32 to 16744bf Compare May 24, 2019 13:47

jbrockmendel force-pushed the typos branch from 16744bf to 8c08c0b Compare May 26, 2019 15:35

jbrockmendel commented May 26, 2019

View reviewed changes

jbrockmendel mentioned this pull request May 27, 2019

CLN: test_data.py #5796

Merged

jbrockmendel force-pushed the typos branch from 8c08c0b to 7e36818 Compare May 29, 2019 16:46

jbrockmendel added 7 commits July 12, 2019 06:20

TST: no cover for test code that is OK not to run

dd22f02

TST: enable/mark mangled or commented-out tests

5a206f2

troubleshoot xpass

1b1e1a6

remove unused import

1ebe848

MAINT: revert no-longer-necessary pragma no covers

7438742

MAINT: add passing files to linted list

8171298

remove unnecessary

6ddafc0

jbrockmendel force-pushed the typos branch from 7e36818 to 6ddafc0 Compare July 12, 2019 13:22

bashtage merged commit c5e665e into statsmodels:master Jul 12, 2019

jbrockmendel deleted the typos branch July 13, 2019 00:46

bashtage added the maintenance label Dec 16, 2019

bashtage added this to the 0.11 milestone Dec 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP/TST: enable/mark mangled/commented-out tests #5768

WIP/TST: enable/mark mangled/commented-out tests #5768

jbrockmendel commented May 21, 2019

pep8speaks commented May 21, 2019 •

edited

Loading

josef-pkt May 21, 2019

jbrockmendel May 21, 2019

josef-pkt May 21, 2019

jbrockmendel May 21, 2019

josef-pkt May 21, 2019

jbrockmendel May 21, 2019

josef-pkt May 21, 2019

josef-pkt commented May 21, 2019

bashtage May 22, 2019

jbrockmendel May 22, 2019

jbrockmendel May 22, 2019

bashtage May 22, 2019

jbrockmendel May 22, 2019

josef-pkt May 22, 2019

bashtage May 22, 2019

bashtage May 22, 2019

jbrockmendel May 22, 2019

jbrockmendel commented May 22, 2019 •

edited

Loading

codecov bot commented May 23, 2019

codecov bot commented May 23, 2019 •

edited

Loading

coveralls commented May 23, 2019 •

edited

Loading

bashtage commented May 23, 2019

bashtage commented May 24, 2019

jbrockmendel commented May 24, 2019

jbrockmendel May 26, 2019

bashtage commented Jul 12, 2019

		@@ -65,61 +65,54 @@ def test_basic(self):


		def test_basic_method(self):

WIP/TST: enable/mark mangled/commented-out tests #5768

WIP/TST: enable/mark mangled/commented-out tests #5768

Conversation

jbrockmendel commented May 21, 2019

pep8speaks commented May 21, 2019 • edited Loading

Comment last updated at 2019-07-12 13:22:53 UTC

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josef-pkt commented May 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented May 22, 2019 • edited Loading

codecov bot commented May 23, 2019

Codecov Report

codecov bot commented May 23, 2019 • edited Loading

Codecov Report

coveralls commented May 23, 2019 • edited Loading

bashtage commented May 23, 2019

bashtage commented May 24, 2019

jbrockmendel commented May 24, 2019

Choose a reason for hiding this comment

bashtage commented Jul 12, 2019

pep8speaks commented May 21, 2019 •

edited

Loading

jbrockmendel commented May 22, 2019 •

edited

Loading

codecov bot commented May 23, 2019 •

edited

Loading

coveralls commented May 23, 2019 •

edited

Loading