convert floating point stats from string - #241
Conversation
|
I've gone ahead and removed the broken workflows and scripts that was causing the CI failure. Can you pull them down for the PR and run it? Also thank you for the PR :) |
212be05 to
36de8bc
Compare
|
Thanks, I merged in your upstream changes, but I can't trigger any workflows. |
8a38c37 to
a14637e
Compare
|
@Mattsface I've rebased on top of 0.8.0 all tests are passing |
|
@claude review please |
|
Claude finished @Mattsface's task in 2m 20s —— View job Code Review
CI (Offline CI) is passing on this PR. Note: I wasn't able to execute the local test suite myself in this sandbox (bash execution requiring approval was denied), so this review is based on static analysis. Summary: Solid, low-risk change. The A few things worth addressing:
None of these block the PR — 1 and 2 are the most worth fixing before merge since they're small, concrete gaps; 3 and 4 are minor polish. |
6ec3daf to
f148f59
Compare
|
I added the OptionalFoat to the missing variables in ExpectedStatistics and updated the stale docs in hitting.py. Regarding innings_pitched, the .1 for 1/3, .2 for 2/3 is the convention used in the API (baseball reference notes it here https://www.baseball-reference.com/bullpen/Inning). I don't think it makes sense to change the value that is reported for innings pitched here, but let me know if you disagree. |
f148f59 to
7c0569a
Compare
|
@louisadamian this is totally on my radar! I will clone it and take a look ASAP. |
|
@louisadamian also yes this is interesting.. "Innings Pitched (IP) measures the length of a pitcher's appearance by outs. If a pitcher gets 1 out it's 0.1 innings (also written 0.3, both meaning 1/3 of an inning), 2 outs is 0.2 innings (also written 0.7) and 0 outs is 0 innings. Until 1982, innings pitched were rounded to the nearest full inning (e.g. 13.2 innings pitched became 14, and 13.1 became 13). This practice caused a controversy in the attribution of the 1981 AL ERA title and was scrapped (see below). The practice had been adopted before the invention of the pocket calculator and the spread of computers and no longer made any sense. Encyclopedias now list full innings pitched, including fractions, for the period before 1982, but older sources of data will often have rounded innings, and as a result pitchers' ERAs will differ slightly." |
|
So I discovered that pydantic will actually handle this string to float conversion for us @louisadamian. I'm working on patch to change the the ones referenced in this PR. I think we should leave that odd return for innings_pitched as a string unless you have any other thoughts. |
Convert floating stats with decimal representation from str to float with pydantic
adds a
BeforeValidatorto base.py that converts types to float or returns none and anOptionalFloattype that used the validator and changed all the relevant types in stat objects.Tests
Passes all unit tests.