Closed shreyapandit closed 1 year ago
That's great @shreyapandit thanks for taking the lead on a not-so-glamorous job!
I wrote the original versions of both standardize_concerns and standardize_crashes. Based on feedback from Jenny I then rewrote sections of standardize_crashes to remove city-specific switching, but haven't similarly reviewed concerns. Also I've completed an online python course since then, so my coding abilities have probably improved quite a bit!
If you have any concerns about them let me know, there is probably quite a bit of scope for improvement.
Thanks.
Shreya, great, thank you! Can I suggest that you do a PR for what you've already done? That way if people make changes to files you're testing, there won't be as much chance for conflicts.
Speaking of which, regarding testing for standardize_crashes/concerns - on my (badly named) branch more_analysis, I've added another standardization script, and as a result, refactored the directory a little to add a util file, and added tests for those new functions. So I'm going to try to get that finished today and put in a PR so if you're writing tests on that, we won't conflict.
Thanks @j-t-t and @terryf82 ! I will look into refactoring the concerns script as well, and will make a pull request soon
The pull request can be found here: https://github.com/Data4Democracy/crash-model/pull/152
Hey Shreya do you need a hand with this? It looks like the PR has some failing tests, though I'm not sure if these are real or just timeouts..
Codecoverage is available!
Added test for running initialize_city, and one for running pipeline before viz. Now, based on codecov feedback, I am working on testing the standardize_concerns and standardize_crashes scripts under data_standardization.