Closed shreyapandit closed 5 years ago
Looks like pylint has failed...will look into it
@shreyapandit there's a few pickle files you're merging here, which probably don't make sense to include. I think, generally, the only things that should be included here are the notebooks. And for that, I don't feel that we need much in the way of CI/coverge checks, it's experimental. Is that reasonable @j-t-t ?
@shreyapandit there's a few pickle files you're merging here, which probably don't make sense to include. I think, generally, the only things that should be included here are the notebooks. And for that, I don't feel that we need much in the way of CI/coverage checks, it's experimental. Is that reasonable @j-t-t ?
Certainly notebooks aren't included in the codecov metrics at this point. If @shreyapandit is writing a pkl file as part of the train_model, I might throw in a test for that but it would be simple/quick to do
Just took a look at the pickle, it looks like it's the same as the seg_w_predicted.csv that comes out of the model. Is that right, Shreya?
On Mon, Oct 22, 2018 at 10:03 AM Jenny Turner-Trauring < notifications@github.com> wrote:
Certainly notebooks aren't included in the codecov metrics at this point. If @shreyapandit https://github.com/shreyapandit is writing a pkl file as part of the train_model, I might throw in a test for that but it would be simple/quick to do
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Data4Democracy/crash-model/pull/207#issuecomment-431845008, or mute the thread https://github.com/notifications/unsubscribe-auth/ANMbgaOv_u78I8qUA68pu1lSHbW0YLg3ks5undBOgaJpZM4XdjwP .
If we're aligned on focusing development on segment-level predictions in the future I think it would be good to update this notebook to reflect that. This still references a lot of week-level properties and I found it a little difficult to follow through for a city (Brisbane) that had segment-level preds.
Why don't we have a conversation about this this week. I think it probably doesn't make sense to try and expand development to other cities until we're comfortable with the methodology.
On Mon, Oct 22, 2018 at 6:08 PM Terry Franklin notifications@github.com wrote:
If we're aligned on focusing development on segment-level predictions in the future I think it would be good to update this notebook to reflect that. This still references a lot of week-level properties and I found it a little difficult to follow through for a city (Brisbane) that had segment-level preds.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Data4Democracy/crash-model/pull/207#issuecomment-432007949, or mute the thread https://github.com/notifications/unsubscribe-auth/ANMbgb_Mk5ieUXcnhtlty-98fSqTCg0gks5unkHKgaJpZM4XdjwP .
Merging #207 into master will increase coverage by
0.03%
. The diff coverage is100%
.
@@ Coverage Diff @@
## master #207 +/- ##
==========================================
+ Coverage 52.21% 52.24% +0.03%
==========================================
Files 29 29
Lines 2959 2961 +2
==========================================
+ Hits 1545 1547 +2
Misses 1414 1414
Closing this for the moment, will reopen once we agree on a standard method to use for all cities.
This PR is a work in progress and will be mainly useful for providing a nice UI to look through the notebooks which pertain to the profile analysis.