Replace geom_smooth(stat = "identity") with geom_line() in ppc-discrete.R

[utkarshpawade]

Fixes #488

Summary

• Replaced geom_smooth(stat = "identity") with geom_line() in R/ppc-discrete.R (line 322)
• geom_smooth() with stat = "identity" bypasses smoothing and is functionally equivalent to geom_line(), but misleads readers into expecting a smoothing curve. See https://ggplot2.tidyverse.org/reference/geom_smooth.html
• Also removed the unused fill argument which has no effect on geom_line()

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.66%. Comparing base (906cd5c) to head (d300be6).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #489      +/-   ##
==========================================
- Coverage   98.66%   98.66%   -0.01%     
==========================================
  Files          35       35              
  Lines        5860     5858       -2     
==========================================
- Hits         5782     5780       -2     
  Misses         78       78              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jgabry]

This change isn't actually functionally equivalent. You can see this immediately if you look at the SVG snapshots that were updated as part of the PR (in the future, if you're expecting the plot to look the same after a changing the code, please check that this is the case before submitting the PR).

The current geom_smooth(stat = "identity") layer is picking up the inherited ymin/ymax added later in the plot construction, so it draws both the expected-count line and the uncertainty band. Replacing it with geom_line() removes that interval layer entirely. It should be possible to switch to geom_line() but you'd need to include also a geom_ribbon() I think. So you could update the PR to do that, or we can close this and keep the current version.

but misleads readers into expecting a smoothing curve

I don't think this misleads users. This code is internal, the user doesn't know if geom_line or geom_smooth is called unless they dig into the source code.

[utkarshpawade]

My bad, I missed that it was also inheriting ymin/ymax for the ribbon, so yeah not equivalent at all. And you're right, it's internal code so the readability argument doesn't really hold. Closing this, makes sense to keep the current version.

[jgabry]

My bad, I missed that it was also inheriting ymin/ymax for the ribbon, so yeah not equivalent at all.

No worries, these things can be pretty subtle sometimes.