Fix for ggplot_3.3.0 scales issue

[rmcd1024]

This implements the fix suggested by @paleolimbot. Tested and it fixes the problem I reported.

[yutannihilation]

This is outside of a function, which means this is determined on installation. Though I guess this works in most cases, there's a chance the user installs ggmisaic when their ggplot2 is version 3.2.1 and then update ggplot2 to version >=3.3.0.

What about setting ggplot2::waiver() as the default and replace it with "none" in scale_(x|y)_productlist() only when the version of ggplot2 is <= 3.2.1?

[paleolimbot]

Thanks for catching that @yutannihilation! Putting that code inside a function will result in fewer accidentally hidden axes when you release the code!

[rmcd1024]

That's an excellent point. Does your suggestion still run into the problem of the user upgrading ggplot? Two other edits come to mind. 1. Define default_guide as a function. I presume this imposes some performance penalty? Hard to imagine it would be significant but I'm curious to know what you think. 2. Use .onLoad. I've never used this capability and don't know if there is anything to be aware of. Any thoughts?

…

On Wed, Mar 18, 2020 at 6:37 PM Hiroaki Yutani ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In R/scale-product.R <#42 (comment)>: > @@ -31,7 +31,12 @@ product_labels <- function() { is.waive <- getFromNamespace("is.waive", "ggplot2") - +## Fix for ggplot_3.3 +if (utils::packageVersion("ggplot2") >= "3.2.1.9000") { This is outside of a function, which means this is determined on installation. Though I guess this works in most cases, there's a chance the user installs ggmisaic when their ggplot2 is version 3.2.1 and then update ggplot2 to version >=3.3.0. What about setting ggplot2::waiver() as the default and replace it with "none" in scale_(x|y)_productlist() only when the version of ggplot2 is <= 3.2.1? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#42 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACBGHVKFHFZ6EOSJ6WFJHJDRIFLM3ANCNFSM4LO35QJQ> .

[paleolimbot]

I think default_guide() as a function is the easiest way, and performance isn't an issue I don't think. The .onLoad() hook is a bit harder to read, but it's how the ggforce package handles this problem if you want to have a look.

[yutannihilation]

I have nothing add to @paleolimbot's comment. Both options sound good to me!

[clauswilke]

ggmosaic could also simply move to requiring ggplot2 3.3.0. It's for the maintainers to decide, but it's what I would probably do.

[haleyjeppson]

Thank you all for the valuable input (and patience while I finished up the semester). I have implemented these ideas (and a little extra) in the latest version.