Skip to content
New issue

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

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

Already on GitHub? # to your account

Add sack player and yards information #180

Closed
mrcaseb opened this issue Feb 9, 2021 · 6 comments · Fixed by #214
Closed

Add sack player and yards information #180

mrcaseb opened this issue Feb 9, 2021 · 6 comments · Fixed by #214
Labels
Feature Request Offseason to-do Something to add when season is over

Comments

@mrcaseb
Copy link
Member

mrcaseb commented Feb 9, 2021

https://github.com/mrcaseb/nflfastR/blob/c3bf6e9ed83e64699bb95078007efdeab012db26/R/helper_tidy_play_stats.R#L963-L964

Currently we only assign sack == 1 for stat ID 83

It would be very easy to add sack_player_name, sack_player_id and sack_yards

Don't forget to look at stat ID 84!

@mrcaseb
Copy link
Member Author

mrcaseb commented Feb 9, 2021

We should also rewrite the definition of qb_hit_*_player. It is assigned for stat ID 110 but it sounds like this is the sack_player

https://github.com/mrcaseb/nflfastR/blob/c3bf6e9ed83e64699bb95078007efdeab012db26/R/top-level_scraper.R#L229-L232

grafik

@TheMathNinja
Copy link
Contributor

I think this could be different in some instances. Not all QB Hits are Sacks, and not all Sacks are QB Hits. In particular, I wonder if there's a play when two defenders arrive at the QB at the same time: one strips the ball (the sacker), and the other impacts the QB strongly enough to put him on the ground (the QB Hitter). You might run into issues on plays like this.

@mrcaseb
Copy link
Member Author

mrcaseb commented Feb 9, 2021

I probably didn't make clear what I want to do. I want to add sack_player if stat ID 83 is present and I want to keep qb_hit_*_player if stat id 110 is present. I just want to rewrite the description text for qb_hit_*_player. I don't see why I should run into issues doing this tbh.

@TheMathNinja
Copy link
Contributor

Ok I guess I just don't understand what is being suggested (especially in terms of how sack_player is being written/defined, and how qb_hit_player might be re-defined). But I'm very excited for this feature of nflfastR!

@mrcaseb mrcaseb added Feature Request Offseason to-do Something to add when season is over labels Feb 9, 2021
@TheMathNinja
Copy link
Contributor

FWIW, I've historically used
sack_yards = if_else(sack==1, -1*yards_gained, 0))

I think this works? Regardless of whether stat id 83 is present or not.

@mrcaseb
Copy link
Member Author

mrcaseb commented Feb 18, 2021

Yep this should work because stat id 20 sets sack == 1 and the corresponding yards to yards_gained.

So probably a sack_yards variable for stat id 83 might be redundant. The player information isn't though.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Feature Request Offseason to-do Something to add when season is over
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants