search

HealthCatalyst / healthcareai-r

R tools for healthcare machine learning
https://docs.healthcare.ai
Other
245 stars 106 forks source link

prep_data doesn't handle logical predictors #1223

Closed glenrs closed 6 years ago

glenrs commented 6 years ago

@michaellevy, I am embarrassed that this took as long as it did, but I learned a lot that is not shown in my code. This ended up being a simple fix. I had done a lot more, but I found that it wasn't important. I believe this has the functionality that this issue is asking for.

codecov[bot] commented 6 years ago

Codecov Report

Merging #1223 into master will increase coverage by <.1%. The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #1223     +/-   ##
========================================
+ Coverage    94.4%   94.4%   +<.1%     
========================================
  Files          37      37             
  Lines        2477    2480      +3     
========================================
+ Hits         2340    2343      +3     
  Misses        137     137
michaellevy commented 6 years ago

Looks great, thanks @glenrs!

For what it's worth, trying a more complicated solution or trying to do more, and then realizing that there's a simpler solution or that a lighter change would be better is a pattern that I engage in regularly. It always feels frustrating because in some sense the work could've been done faster, but apparently I needed that time to learn what the better solution is. So, don't fret when this happens; it can be a positive, both for our code base and our learning as developers.

glenrs commented 6 years ago

Thank you!

From: Michael Levy notifications@github.com Reply-To: HealthCatalyst/healthcareai-r reply@reply.github.com Date: Wednesday, August 15, 2018 at 11:29 AM To: HealthCatalyst/healthcareai-r healthcareai-r@noreply.github.com Cc: Rex Sumsion rex.sumsion@healthcatalyst.com, Mention mention@noreply.github.com Subject: Re: [HealthCatalyst/healthcareai-r] prep_data doesn't handle logical predictors (#1223)

Looks great, thanks @glenrshttps://github.com/glenrs!

For what it's worth, trying a more complicated solution or trying to do more, and then realizing that there's a simpler solution or that a lighter change would be better is a pattern that I engage in regularly. It always feels frustrating because in some sense the work could've been done faster, but apparently I needed that time to learn what the better solution is. So, don't fret when this happens; it can be a positive, both for our code base and our learning as developers.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/HealthCatalyst/healthcareai-r/pull/1223#issuecomment-413272653, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AckgSs01nKCqD7m1jbbVYL9mVWPRG-E8ks5uRFqDgaJpZM4V8zzw.