Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Aug 6, 2022

Conversation

tjburch
Copy link
Collaborator

@tjburch tjburch commented Jul 31, 2022

One line change, addressing the FutureWarning raised in #270

@tjburch
Copy link
Collaborator Author

tjburch commented Jul 31, 2022

Failing tests should be fixed once #280 is merged.

@schorrm
Copy link
Collaborator

schorrm commented Aug 4, 2022

Can you merge up again for CI?

@tjburch
Copy link
Collaborator Author

tjburch commented Aug 4, 2022

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
Copy link
Collaborator Author

tjburch commented Aug 6, 2022

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 schorrm merged commit e57e6e7 into jldbc:master Aug 6, 2022
@schorrm
Copy link
Collaborator

schorrm commented Aug 6, 2022

Great job as always, tyvm

@tjburch tjburch deleted the drop-keyword branch December 2, 2022 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants