Skip to content

Conversation

@OkonSamuel
Copy link
Member

@OkonSamuel OkonSamuel commented Sep 30, 2020

Previously tests on linear models were carried out using matrix input these lead to a bug in the table output gotten from these models . This PR fixes this by

  1. testing linear and ridge regression with table input instead of matrix input.

  2. adding tests for multi-target linear and ridge regression to ensure features of predicted target table and training target table are same
    cc.@ablaom.

…x input.

2. For multi-target linear and ridge regression add tests to ensure features of predicted target table and training target table are same
@codecov-commenter
Copy link

Codecov Report

Merging #5 into master will increase coverage by 0.13%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #5      +/-   ##
==========================================
+ Coverage   85.71%   85.84%   +0.13%     
==========================================
  Files           5        5              
  Lines         217      219       +2     
==========================================
+ Hits          186      188       +2     
  Misses         31       31              
Impacted Files Coverage Δ
src/models/linear_models.jl 95.00% <92.85%> (+0.26%) ⬆️

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 1bed102...9b3e341. Read the comment docs.

Copy link
Member

@ablaom ablaom left a comment

Choose a reason for hiding this comment

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

This looks good, many thanks!!

However, it seems that more than tests are being altered here. Is the new src/ code just a cleanup or a genuine bug fix?

@OkonSamuel
Copy link
Member Author

it's a bug fix that was detected when i updated the tests.

@ablaom ablaom merged commit 3261df6 into JuliaAI:master Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants