optional arguments for logistic_regression() in mediation_analysis()

[julibeg]

As discussed in #244.

I added a dict logreg_kwargs with the arguments for logistic regression instead of **kwargs as this might be cleaner at some later time when you might also want to pass arguments to other functions called by mediation_analysis() (e.g. linear_regression()). If you prefer the **kwargs solution, let me know and I'll change it.

Also, I'm not really familiar with Sphinx, please check if the docstring correct.

[codecov]

Codecov Report

Merging #245 (0d5bb72) into master (367c935) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #245   +/-   ##
=======================================
  Coverage   98.99%   98.99%           
=======================================
  Files          19       19           
  Lines        3286     3287    +1     
  Branches      523      523           
=======================================
+ Hits         3253     3254    +1     
  Misses         17       17           
  Partials       16       16           

Impacted Files	Coverage Δ
pingouin/regression.py	100.00% <100.00%> (ø)

---

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 367c935...0d5bb72. Read the comment docs.

[raphaelvallat]

Hi @julibeg,

That looks great, thanks! Have you made sure that changing the logreg_kwargs change the output of the mediation analysis? Could you perhaps add just one test in the test_regression.py file?

Thanks,
Raphael

[julibeg]

I made sure by printing the kwargs right before lom.fit(X, y) in logistic_regression() (i.e. here). Will look at the tests later. But in theory the results shouldn't really change when the problem is well behaved I guess.

[julibeg]

Sorry for the delay. I added some tests that check if an error is thrown with logreg_kwargs=dict(max_iter=-1) and that compare results when running mediation_analysis with logreg_kwargs=dict(max_iter=0).

Something else I noticed: When testing the indirect effect with assert_almost_equal(ma['coef'][4], 0.0033, decimal=2), 2 might not be strict enough for the test to be meaningful. Maybe replace it with 3.

[raphaelvallat]

Hi @julibeg,

Thanks! I agree we can use decimal=3. However, can you explain what these added lines do:

 with pytest.raises(AssertionError):
    assert_almost_equal(ma['coef'][0], -0.0208, decimal=2)
with pytest.raises(AssertionError):
    assert_almost_equal(ma['coef'][4], 0.0033, decimal=3)

Right now, this will test that ma['coef'][0] is not equal to -0.0208, and as a result throw an assertion error. Is this the correct behavior?

Thank you
Raphael

[julibeg]

# Solve with 0 iterations and make sure that the results are different
ma = mediation_analysis(data=df, x='X', m='Mbin', y='Y', n_boot=2000,
                        logreg_kwargs=dict(max_iter=0))

This runs the mediation analysis while not allowing the solver to iterate so that the results are different (however, weirdly not all the coefs are different, just coef[0] and coef[4]).

with pytest.raises(AssertionError):
    assert_almost_equal(ma['coef'][0], -0.0208, decimal=2)
with pytest.raises(AssertionError):
    assert_almost_equal(ma['coef'][4], 0.0033, decimal=3)

This checks that the coefs are indeed different and that the assertion fails.

[raphaelvallat]

Got it, thanks! So are we ready to merge?

[julibeg]

I think so