Update suggests

[nikosbosse]

The R package distributional (https://github.com/mitchelloharawild/distributional) saw an update on September 17. Unfortunately, they are now using the native pipe |> in one or two places.
We have ggdist as a suggest which in turn imports distributional which in turn means that scoringutils isn't fully compatible with R version 4.0 anymore.

This PR simply comments out the code that requires ggdist in an old vignette, which is a bit unfortunate. The alternative would be to open an issue with distributional and kindly ask them whether they could use %>% instead.

We should probably merge this PR regardless as it would take them some time to use %>% even if they were willing to do so.

[sbfnk]

The alternative would be to open an issue with distributional and kindly ask them whether they could use %>% instead.

See mitchelloharawild/distributional#128

[nikosbosse]

Always 3 steps ahead of me...

[Bisaloo]

Since it's a Suggests, I'm not sure this change requires its removal. My first idea would be to make these chunks run optionally on the presence of ggdist and keep it in Suggests;

[nikosbosse]

But don't we get a note regardless if there is a :: without a suggest?

EDIT: We do, at least the way I tried it (setting the Rmd chunk to evaluate depending on whether ggidst is installed)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.33%. Comparing base (3d4a0ce) to head (ff33cc7).
Report is 24 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #917      +/-   ##
==========================================
+ Coverage   98.96%   99.33%   +0.36%     
==========================================
  Files          22       22              
  Lines        1647     1650       +3     
==========================================
+ Hits         1630     1639       +9     
+ Misses         17       11       -6     

Flag	Coverage Δ
	99.33% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[seabbs]

The alternative is we simply bump our minimum R version? That doesn't seem like the worst thing in the world? Happy to comment out for now if others don't think this is a good idea and none of the suggested alternatives pan out.

[nikosbosse]

Upside would be that we could also start using the native pipe everywhere. Downside is that people might find it hard to install the latest R version on systems where they aren't the admin.
Bit of a judgement call. I find it hard to tell how many people would find it difficult to upgrade to R 4.1.
@Bisaloo what do you think?

[Bisaloo]

But don't we get a note regardless if there is a :: without a suggest?

Yes, the proposal was to keep ggdist in Suggests. This means this specific vignette won't render for users on R 4.0 but they can still use the package normally. This seems like the path with the least amount of friction

[nikosbosse]

Yes, the proposal was to keep ggdist in Suggests. This means this specific vignette won't render for users on R 4.0 but they can still use the package normally. This seems like the path with the least amount of friction

Ah right - then we would just remove R 4.0. from the CI and be done?

[seabbs]

We would bump the minimum version in the description and ci to match distributional

[nikosbosse]

We would bump the minimum version in the description and ci to match distributional

I think that's not what @Bisaloo was suggesting and I don't think we actually need that

[Bisaloo]

We would skip the install of ggdist on R 4.0. This is not a problem if the vignette chunks are set up to run conditionally on ggdist presence.

https://github.com/r-lib/actions/tree/v2-branch/setup-r-dependencies#ignoring-optional-dependencies-that-need-a-newer-r-version

[seabbs]

No I know it's not what they suggested it's the alternative if nothing else works and I think is better than commenting out.

[nikosbosse]

I think Hugo's suggestion worked - ready to merge as far as I'm concerned

[sbfnk]

Just noting that this has been fixed in {distributional} so the optional dependency could be removed once that's on CRAN.

[nikosbosse]

Nice, I referenced this in a new issue