Closed wfordh closed 3 years ago
Looks like a good initial draft here -- ideally, I would prefer to bind the parameters for the OAA leaderboard to the func call, rather than just dangle in the thingy with kwargs.
This is a design decision though -- other than this point, the rest looks ready to move on to testing + documentation, this decision should be made and then
Yeah it seems like some of the args are mutually exclusive so that's why I suggested kwargs, but I think keeping it straightforward for now is best. Can revisit if people want that additional flexibility.
Will get started on the tests and docs, and pull in those other changes.
@schorrm LMK how it looks when you get a chance
Sorry for the nutso review, but this is big. Great work, even if the review is only about quibbling. Sorry.
No worries at all! Better to cover all of it now
Oops, had an issue with tabs / spaces in utils.py
that caused a big "change", but only material change there is at the beginning of norm_positions()
Pulling data from statcast's fielding and running leaderboards. Can change to fielder and runner if that makes more sense / is more consistent. Most of it was pretty straightforward except for
statcast_outs_above_average()
due to how the positions are handled - if there's a better way to donorm_positions()
, I'm all ears.The OAA leaderboard in particular has a lot of options and using
kwargs
so the user can supply more args might be a good next step after this.There's also no catcher pop time data for 2020, which I found odd. Once all of this checks out I will start on the documentation and tests.