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

Whiff % is showing 0% for all pitches #59

Closed
kruser opened this issue Apr 6, 2014 · 3 comments
Closed

Whiff % is showing 0% for all pitches #59

kruser opened this issue Apr 6, 2014 · 3 comments
Labels

Comments

@kruser
Copy link
Owner

kruser commented Apr 6, 2014

Ughh

image

@kruser kruser added the bug label Apr 6, 2014
@albertlyu
Copy link
Contributor

Looks like pitchStats.js was missing:

if (pitch.isWhiff())
{
    aggregator.whiff++;
}

I'm not sure why. I'll investigate. I'm guessing I didn't have an updated local repo when I submitted my pull request.

@albertlyu
Copy link
Contributor

See commit 5e0f497 for the last time whiffs were being aggregated.

if (pitch.isSwing())
{
aggregator.swing++;
/*jshint maxdepth:5 */
if (pitch.isWhiff())
{
aggregator.whiff++;
if ((!havePitchTypeFilters || pitchTypeFilters[pitchCode]) && angular.isDefined(pitch.px) && angular.isDefined(pitch.pz))
{
whiffSeries.push([pitch.px, pitch.pz]);
}
}
}

When ec62105 was pushed to master, the whiff stats were wiped out. I believe the intention was to remove the whiffSeries object, but in doing so, aggregator.whiff++; was also removed. 5e0f497...ec62105#diff-112bb635c769a65c5ead2fd78f47975aR210

I believe the fix would be to add the following to L330-L333 at https://github.com/kruser/pitchfx-site/blob/master/app/app/scripts/controllers/pitchStats.js#L330-L333:

if (pitch.isSwing()) {
    aggregator.swing++;
    if (pitch.isWhiff()) {
        aggregator.whiff++;
    }
}

I will submit a pull request with these changes later today, for @kruser's review.

albertlyu added a commit to albertlyu/pitchfx-site that referenced this issue Apr 6, 2014
@kruser kruser closed this as completed in 79d4503 Apr 6, 2014
@kruser
Copy link
Owner Author

kruser commented Apr 6, 2014

deploying to site now. I only see you on the site @albertlyu so it's safe to deploy.

FYI, if you don't want to show up on my Google Analytics you can always opt-out on Chrome. That's what I do.
https://tools.google.com/dlpage/gaoptout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants