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

Invalid handling of Joint positions in mfl_starterpositions #399

Open
mcarman8 opened this issue Jun 28, 2023 · 3 comments
Open

Invalid handling of Joint positions in mfl_starterpositions #399

mcarman8 opened this issue Jun 28, 2023 · 3 comments

Comments

@mcarman8
Copy link
Contributor

Describe the bug
joint positions such as WR+TE are handled incorrectly. The result of ff_starter_positions is
pos | min | max
WR | 1 | 1
TE | 1 | 1
When it should be
pos | min | max
WR | 0 | 1
TE | 0 | 1

The offending line seems to be the
tidyr::separate_rows("pos",sep = "+")
At line 50, as that will keep the value for the minimum as 1 when the minimum for those positions is technically 0.

For defense, there is an additional potential problem that could happen even as a result of this fix.
Since there is DT+DE and CB+S, a valid conversion of 1 of each position would be
pos | min | max | defense_starters
DT | 0 | 1 | 2
DE | 0 | 1 | 2
CB | 0 | 1 | 2
S | 0 | 1 | 2

However, reading this would imply that a DT and a DE would be valid result, but only 1 from that group is actually valid via the MFL scoring settings. The only valid way to keep all of this information is to either pass WR+TE, DT+DE, and CB+S as a position, or to add additional columns that describe minimum and maximum limits for the groupings.

Reprex
Create an MFL league and set the positions to 1 WR+TE, 1 DT+DE, and 1 CB+S position. While the free trial is going, 19740 has these options set.

 mfl_connect(2023, league_id = 19740)
 positions <- ff_starter_positions(conn)

Expected behavior
The positions table has 0 for the minimum of every position instead of the current result of 1.

Session information
Please copy the output of ffscrapr::ffverse_sitrep() into the box below.

You've asked for the packages ffpros and ffopportunity which are not installed. 
They are skipped.
── System Info ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
• R version 4.3.0 (2023-04-21 ucrt)   • Running under: Windows 10 x64 (build 19045)
── ffverse Packages ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
• ffscrapr    (1.4.8)
• ffsimulator (1.2.3)
── ffverse Options ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
No options set for ffscrapr and ffsimulator
── ffverse Dependencies ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
• askpass    (1.1)     • httr       (1.4.6)  • Rglpk      (0.6-5)   
• assertthat (0.2.1)   • jsonlite   (1.8.4)  • rlang      (1.1.1)   
• backports  (1.4.1)   • lifecycle  (1.0.3)  • slam       (0.1-50)  
• cachem     (1.0.8)   • magrittr   (2.0.3)  • stringi    (1.7.12)  
• checkmate  (2.2.0)   • memoise    (2.0.1)  • stringr    (1.5.0)   
• cli        (3.6.1)   • mime       (0.12)   • sys        (3.4.1)   
• cpp11      (0.4.3)   • nflreadr   (1.3.2)  • tibble     (3.2.1)   
• curl       (5.0.0)   • openssl    (2.0.6)  • tidyr      (1.3.0)   
• data.table (1.14.8)  • pillar     (1.9.0)  • tidyselect (1.2.0)   
• dplyr      (1.1.2)   • pkgconfig  (2.0.3)  • tidytable  (0.10.1)  
• fansi      (1.0.4)   • purrr      (1.0.1)  • utf8       (1.2.3)   
• fastmap    (1.1.1)   • R6         (2.5.1)  • vctrs      (0.6.2)   
• generics   (0.1.3)   • rappdirs   (0.3.3)  • withr      (2.5.0)   
• glue       (1.6.2)   • ratelimitr (0.4.1)    
──────────────────────────────────────────────────────────────
@tanho63
Copy link
Member

tanho63 commented Jun 28, 2023

Yeah I’ve been thinking about this one since you mentioned it. Here are the official documentation notes for how MFL uses combo positions: https://www46.myfantasyleague.com/2023/support?L=22627&CATEGORY=Players%20%26%20Lineups&SUBCATEGORY=Lineup%20Setup

One important point - you should only use the combine positions (RB+WR+TE, or WR+TE) if your league truly considers all of those positions exactly the same, without differentiating them at all. In other words, you should not use the combined RB+WR+TE option to set up a "flex" position - this option is only to be used if it truly does not matter at all if all players at any of these positions (RB, WR, TE) can be interchanged for each other in your starting lineup rules - for example, if starting 5 TEs in your lineup is allowed, instead of a more traditional lineup (2 RB, 2WR, 1TE).

@tanho63
Copy link
Member

tanho63 commented Jun 28, 2023

With that in mind and also your comments about DT+DE etc, I’m currently thinking the right approach is to add the option to split the positions as an argument (defaulting to split = TRUE to preserve current behaviour), and then setting any combo positions to a minimum of zero if the split process happens

@mcarman8
Copy link
Contributor Author

I think that works best, with maybe a note on the defensive edge case in the documentation.

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

No branches or pull requests

2 participants