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

Remove remaining uses of getFromNamespace() #45

Closed
willgearty opened this issue Oct 18, 2022 · 3 comments
Closed

Remove remaining uses of getFromNamespace() #45

willgearty opened this issue Oct 18, 2022 · 3 comments
Assignees

Comments

@willgearty
Copy link
Owner

New guidelines recommend to not use ::: or getFromNamespace: https://www.tidyverse.org/blog/2022/09/playing-on-the-same-team-as-your-dependecy/. I've already fixed the ones that caused issues for ggplot2 version 3.4.0, but the rest should be fixed to prevent future issues.

@willgearty willgearty self-assigned this Oct 18, 2022
@willgearty
Copy link
Owner Author

One of the big ones for this is render_axis() from ggplot2, which is used to render the ticks and axis line in coord_geo(); however, it seems to be a black hole of other unexported functions. Maybe @paleolimbot or @thomasp85 would have suggestions. I'm still learning the various parts of the ggplot2 API, so definitely let me know if I'm just missing something obvious.

@thomasp85
Copy link

The best is of course to not use any unexported functions at all. However, the lesser evil of using unexported functions is to wrap it in a function like so:

render_axis <- function(...) {
  asNamespace("ggplot2")$render_axis(...)
}

You can perhaps avoid this altogether by first calling the parent function of the Coord and then selectively alter the return value - something like this (untested)

  render_axis_h = function(self, panel_params, theme) {
    axes <- CoordTrans$render_axis_h(panel_params, theme)
    if (any(self$pos %in% c("top", "t"))){
      axes$top = render_geo_scale(self, panel_params, theme, "top")
    }
    if (any(self$pos %in% c("bottom", "b"))){
      axes$bottom = render_geo_scale(self, panel_params, theme, "bottom")
    }

    axes
  }

@willgearty
Copy link
Owner Author

Thanks, that was very helpful @thomasp85! This is now fixed in b7044fc

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

No branches or pull requests

2 participants