Range functions should be exported

[clauswilke]

I think the functions in https://github.com/tidyverse/ggplot2/blob/master/R/range.r should be exported. They are needed by anybody who wants to write their own scale functions from scratch.

Possibly they would fit better into the scales package, but since they depend on ggproto there's a dependency problem.

[hadley]

I think we could easily rewrite to remove the dependency issue.

[dpseidel]

Isn't this already done in scales: https://github.com/r-lib/scales/blob/master/R/range.r ?

[clauswilke]

Indeed, looks like that is the case already. Is there any reason why ggplot2 shouldn’t just use those?

[hadley]

I guess we forgot?

@dpseidel do you want to do a PR?

[dpseidel]

Sure thing. I'll work one up this week.

[dpseidel]

@clauswilke, unfortunately once I took a proper look, I realized the range functions in scales were not ready for primetime. I've just opened a PR in scales to fix them in the dev version but since scales 1.0.0 hit CRAN this morning, to avoid major headaches, bringing these into ggplot2 will need to wait until after the next scales release whenever that is.

[paleolimbot]

@dpseidel Are the range functions now safe to use for ggplot2?

[dpseidel]

The situation remains the same: the range functions are fixed in the dev version of scales but we have not had a new release. Now that my dissertation is (all but) finished, I've been meaning to give scales some more attention. Perhaps we can try for a release by end of summer and get this fixed.

[karawoo]

It looks like this should be ready now that scales 1.1.0 was released last fall, @dpseidel is that right?

[dpseidel]

@karawoo Should be! The updated range functions were released with scales 1.1.0

[dpseidel]

@karawoo Tried to build a PR for this just now and am kicking myself because I've discovered scales::DiscreteRange does not currently support the na.rm argument to train_discrete which is critical. I will open a PR to scales now.

In lieu of waiting for a new scales release to make this switch, we could still eliminate the ggproto dependencies now but we will need to include a custom DiscreteRange object with the correct train function. I've made and tested that change here.

Is it a priority to make this switch now? Or should we continue to wait for this to be completely patched in scales?

[clauswilke]

How about making ggplot2 temporarily dependent on the development version of scales? In this way, we can properly test that the two packages work together as expected.

[thomasp85]

I think that is fine. We are a long way from any release right now

[karawoo]

Works for me. I also don't think it's a huge priority to make the switch now, I just happened to see this issue was still open.