Tests for API, more input checks

[tommyod]

• Using ALLOWED_DISTRS instead of re-defining allowed distributions in the context of testing.
• Added more sanitation of user inputs to fit and score.
• Wrote some tests to solidify the API.

[codecov-io]

Codecov Report

Merging #284 into master will increase coverage by 0.41%. The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   73.74%   74.16%   +0.41%     
==========================================
  Files           4        4              
  Lines         617      627      +10     
  Branches      124      128       +4     
==========================================
+ Hits          455      465      +10     
  Misses        123      123              
  Partials       39       39

Impacted Files	Coverage Δ
pyglmnet/pyglmnet.py	80.08% <86.66%> (+0.4%)	:arrow_up:

---

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 a80848c...cf2590e. Read the comment docs.

[tommyod]

Thank you for the prompt review @jasmainak ! I've addressed some of the comments in a commit, and would like to discuss the rest.

The main reason why I added the smoke-tests was because I was figuring out the API, and I wanted to solidify it in the code somewhere. It was hard to get started with the code, since:

• The code in the master README does not run with code from the master.
• There are no short examples in docstrings showing usage.
• There's an examples folder, but the examples are very long and contain dependencies, i.e. from spykes.ml.strf import STRF.

Adding API usage as a that might have been the wrong approach. Instead, I propose:

• Adding 1-2 short examples showing usage as doctests in the GLM object, and running these on travis CI.
• Adding a few short examples in examples and also running these on travis CI.

Running examples on travis CI is important, if not they might diverge from the code in the future. Thoughts on the approach sketched above?

[jasmainak]

The code in the master README does not run with code from the master.

PR to fix this would be welcome :)

There are no short examples in docstrings showing usage.

again, PR is welcome!

There's an examples folder, but the examples are very long and contain dependencies, i.e. from spykes.ml.strf import STRF

yes, I know it's a little unfortunate. I am also not sure know how much spykes is maintained. Do you know other data that could be used to demonstrate the same functionality? Tikhonov regularization in this case ...

Running examples on travis CI is important, if not they might diverge from the code in the future.

Currently we do have CircleCI for examples. I'm not quite sure why it's not getting triggered on PRs.

[jasmainak]

okay I think I fixed the CircleCI issue. If you make a push, it should run CircleCI

[tommyod]

@jasmainak , I've:

• Updated the README example, it runs now.
• Added a tiny doctest. More can be added later.
• Merged some of the small tests.

I believe this PR is good to go. CircleCI is failing, seemingly because plot_group_lasso.py takes too long. Seems unrelated to this PR.

[tommyod]

• Changed input to fit to only allow ndarrays. Simplified validation and the tests.
• Used RandomState instead of np.random.seed. Can you check if it's correct now?
• Regarding the comment about the coefficients after fitting not being equal to those in the input- can it be due to regularization and the optimization routine(e.g. the number of iterations run and the tolerance)? The difference in the coefficients is kind of large though.

[jasmainak]

Merged @tommyod . Thanks!

Let's iterate on the issue with the coefficient in another PR -- it seems unrelated to the changes in this PR.

I had to squash because you had a merge commit in your history. Next time rebase instead of merging or pulling to avoid that.