jldbc / pybaseball

Pull current and historical baseball statistics using Python (Statcast, Baseball Reference, FanGraphs)
MIT License
1.19k stars 324 forks source link

Use columns keyword rather than no-keyword axis for drop in team_results #282

Closed tjburch closed 1 year ago

tjburch commented 1 year ago

One line change, addressing the FutureWarning raised in #270

tjburch commented 1 year ago

Failing tests should be fixed once #280 is merged.

schorrm commented 1 year ago

Can you merge up again for CI?

tjburch commented 1 year ago

Ugh, looks like something stale in the CI?

def test_amateur_draft_future() -> None:
        result = amateur_draft(most_recent_season() + 1, 1, keep_stats=False)

        assert result is not None
        assert not result.empty

        print(result)

        assert len(result.columns) == 8
>       assert len(result) in ([36](https://github.com/jldbc/pybaseball/runs/7680452213?check_suite_focus=true#step:5:37), [37](https://github.com/jldbc/pybaseball/runs/7680452213?check_suite_focus=true#step:5:38))  # the Astros getting docked a pick throws this off for 2021
E       assert 39 in (36, 37)
E        +  where 39 = len(    OvPck            Tm  ... Type                                     Drafted Out of\n0       1       Orioles  ...   HS...xville, TN)\n[38](https://github.com/jldbc/pybaseball/runs/7680452213?check_suite_focus=true#step:5:39)     [39](https://github.com/jldbc/pybaseball/runs/7680452213?check_suite_focus=true#step:5:40)        Padres  ...   HS                              McQueen HS (Reno, NV)\n\n[39 rows x 8 columns])

Can look later about what's going on with that test.

tjburch commented 1 year ago

Yeah ok, so this test is broken since the draft. It assumes that the number of players in the first round is either 36 or 37, but that's going to get thrown off by comp picks and competitive balance picks and the like.

This year there were 39, which is accurately returned (below). I think what we can confidently say is that the number should be larger than 30 (1 per team) and probably less than like 60 or so? Seems conservative on the upper bound. I'll put that in for now and if there's objections, happy to change it.

In [10]: result
Out[10]:
    OvPck            Tm Signed  ...  Pos Type                                     Drafted Out of
0       1       Orioles    NaN  ...   SS   HS                     Stillwater HS (Stillwater, OK)
1       2  Diamondbacks    NaN  ...   OF   HS            Wesleyan School (Peachtree Corners, GA)
2       3       Rangers    NaN  ...    P  NaN                                                NaN
3       4       Pirates    NaN  ...   SS   HS                              Mays HS (Atlanta, GA)
4       5     Nationals      Y  ...   OF   HS                        IMG Academy (Bradenton, FL)
5       6       Marlins      Y  ...   3B  4Yr       Louisiana State University (Baton Rouge, LA)
6       7          Cubs      Y  ...    P  4Yr                University of Oklahoma (Norman, OK)
7       8         Twins    NaN  ...   SS  4Yr  California Polytechnic State University, San L...
8       9        Royals      Y  ...   OF  4Yr  Virginia Polytechnic Institute and State Unive...
9      10       Rockies    NaN  ...    P  4Yr                   Gonzaga University (Spokane, WA)
10     11          Mets    NaN  ...    C  4Yr      Georgia Institute of Technology (Atlanta, GA)
11     12        Tigers    NaN  ...   2B  4Yr                Texas Tech University (Lubbock, TX)
12     13        Angels      Y  ...   SS  4Yr              Campbell University (Buies Creek, NC)
13     14          Mets      Y  ...   SS   HS                      Rockwall-Heath HS (Heath, TX)
14     15        Padres    NaN  ...    P   HS                             Buford HS (Buford, GA)
15     16     Guardians    NaN  ...   OF  4Yr        James Madison University (Harrisonburg, VA)
16     17      Phillies    NaN  ...   OF   HS                   Bishop Gorman HS (Las Vegas, NV)
17     18          Reds    NaN  ...   3B   JC                     Chipola College (Marianna, FL)
18     19     Athletics    NaN  ...    C  4Yr                 University of Arizona (Tucson, AZ)
19     20        Braves      Y  ...    P   HS           Riverside-Brookfield HS (Brookfield, IL)
20     21      Mariners      Y  ...   SS   HS                   North Allegheny HS (Wexford, PA)
21     22     Cardinals    NaN  ...    P  4Yr            Oregon State University (Corvallis, OR)
22     23     Blue Jays    NaN  ...    P   HS              American Heritage HS (Plantation, FL)
23     24       Red Sox    NaN  ...   SS   HS                    Orange Lutheran HS (Orange, CA)
24     25       Yankees    NaN  ...   OF  4Yr              Vanderbilt University (Nashville, TN)
25     26     White Sox      Y  ...    P   HS                        Oswego East HS (Oswego, IL)
26     27       Brewers      Y  ...   SS  4Yr           Coastal Carolina University (Conway, SC)
27     28        Astros    NaN  ...   OF  4Yr            University of Tennessee (Knoxville, TN)
28     29          Rays    NaN  ...   1B   HS                 East Forsyth HS (Kernersville, NC)
29     30        Giants    NaN  ...  TWP  4Yr             University of Connecticut (Storrs, CT)
30     31       Rockies    NaN  ...   OF  4Yr            University of Florida (Gainesville, FL)
31     32          Reds      Y  ...   3B   HS           Westminster Christian School (Miami, FL)
32     33       Orioles      Y  ...   OF  4Yr  University of California, Berkeley (Berkeley, CA)
33     34  Diamondbacks    NaN  ...    P  4Yr  Mississippi State University (Mississippi Stat...
34     35        Braves      Y  ...    P   HS              Bainbridge HS (Bainbridge Island, WA)
35     36       Pirates      Y  ...    P  4Yr              Campbell University (Buies Creek, NC)
36     37     Guardians    NaN  ...    P  4Yr         Oklahoma State University (Stillwater, OK)
37     38       Rockies    NaN  ...   OF  4Yr            University of Tennessee (Knoxville, TN)
38     39        Padres    NaN  ...    P   HS                              McQueen HS (Reno, NV)

[39 rows x 8 columns]

In [11]: len(result)
Out[11]: 39
schorrm commented 1 year ago

Great job as always, tyvm