jimjam-slam / ggflags

A flag geom for ggplot2. Tweaks the original by using round flags (great for plotting as points).
92 stars 15 forks source link

Several errors when attempting to integrate with other packages #26

Closed rempsyc closed 1 year ago

rempsyc commented 1 year ago

I am trying to integrate ggflags within another package, but so far I am reaching some difficulties. Even though I refer to the function by namespace (ggflags::geom_flag), I am getting errors relating to the inability to find underlying functions with no namespace.

Reprex for not finding ggplot2::layer:

test_flags <- function(data) {
  p <-
    ggplot2::ggplot(data, ggplot2::aes(.data$x, .data$y, country = .data$country)) +
    ggflags::geom_flag(size = 8.5)
  p
}

set.seed(1234)
d <- data.frame(
  x = rnorm(50), y = rnorm(50),
  country = sample(c("ar", "fr", "nz", "gb", "es", "ca", "lv", "qa"), 50, TRUE),
  stringsAsFactors = FALSE
)
test_flags(d)
#> Error in layer(geom = GeomFlag, mapping = mapping, data = data, stat = stat, : could not find function "layer"

Created on 2023-09-10 with reprex v2.0.2

Even when adding ggplot2::layer, we are faced with another error about the Geom object not being found:

layer <- ggplot2::layer
test_flags <- function(data) {
  p <-
    ggplot2::ggplot(data, ggplot2::aes(.data$x, .data$y, country = .data$country)) +
    ggflags::geom_flag(size = 8.5)
  p
}

set.seed(1234)
d <- data.frame(
  x = rnorm(50), y = rnorm(50),
  country = sample(c("ar", "fr", "nz", "gb", "es", "ca", "lv", "qa"), 50, TRUE),
  stringsAsFactors = FALSE
)
test_flags(d)
#> Error in eval(`_inherit`, env, NULL): object 'Geom' not found

Created on 2023-09-10 with reprex v2.0.2

When I look at the code, I indeed do not see where the Geom object is defined... But I think it refers to ggplot2::Geom, so adding that, we still cannot find object '.flaglist':

layer <- ggplot2::layer
Geom <- ggplot2::Geom
test_flags <- function(data) {
  p <-
    ggplot2::ggplot(data, ggplot2::aes(.data$x, .data$y, country = .data$country)) +
    ggflags::geom_flag(size = 8.5)
  p
}

set.seed(1234)
d <- data.frame(
  x = rnorm(50), y = rnorm(50),
  country = sample(c("ar", "fr", "nz", "gb", "es", "ca", "lv", "qa"), 50, TRUE),
  stringsAsFactors = FALSE
)
test_flags(d)
#> Error in h(simpleError(msg, call)): error in evaluating the argument 'object' in selecting a method for function 'grobify': object '.flaglist' not found

Created on 2023-09-10 with reprex v2.0.2

And this time I cannot retrace what package it comes from.

Even using @import ggflags, we are faced with another error:

#' @title Generate a waffle plot made of country flags
#' @param df The processed dataframe of data
#' @export
#' @examples
#' set.seed(1234)
#' d <- data.frame(
#'   x = rnorm(50), y = rnorm(50),
#'   country = sample(c("ar", "fr", "nz", "gb", "es", "ca", "lv", "qa"), 50, TRUE),
#'   stringsAsFactors = FALSE
#' )
#' test_flags(d)
#' @import ggflags
test_flags <- function(data) {
  p <-
    ggplot2::ggplot(data, ggplot2::aes(.data$x, .data$y, country = .data$country)) +
    ggflags::geom_flag(size = 8.5)
  p
}

> test_flags(d)
Error in eval(`_inherit`, env, NULL) : object '`_inherit`' not found

Similar result by loading ggflags' namespace:

requireNamespace("ggflags")
#> Loading required namespace: ggflags

layer <- ggplot2::layer
Geom <- ggplot2::Geom
test_flags <- function(data) {
  p <-
    ggplot2::ggplot(data, ggplot2::aes(.data$x, .data$y, country = .data$country)) +
    ggflags::geom_flag(size = 8.5)
  p
}

set.seed(1234)
d <- data.frame(
  x = rnorm(50), y = rnorm(50),
  country = sample(c("ar", "fr", "nz", "gb", "es", "ca", "lv", "qa"), 50, TRUE),
  stringsAsFactors = FALSE
)
test_flags(d)
#> Error in h(simpleError(msg, call)): error in evaluating the argument 'object' in selecting a method for function 'grobify': object '.flaglist' not found

Created on 2023-09-10 with reprex v2.0.2

The only solution seems to literally load ggflags with library(ggflags), but of course this is not recommended practice and will not be eventually accepted on e.g., CRAN. Any idea how to fix this on my side?

jimjam-slam commented 1 year ago

Hi @rempsyc! I think the immediate problem is that the flags are saved as the variable .flaglist inside the RDA file, flags.rda. So most of the examples show running data(lflags) to make .flaglist available in the global environment.

This is probably why you're finding that you explicitly need to load the package in order to trace things. You may still be able to access .flaglist by running data(lflags) (or even data(lflags, "ggflags")) without loading the whole package, but I agree that it isn't great—and given ggflags references .flaglist inside geom_flag without namespacing it, that feels like a problem.

Tbh I haven't touched the flag storing code much since I took over the package, and I think the package could use a bit of an overhaul generally given how much the SVG landscape has changed in the last 5-6 years (but it also hasn't been at the top of my priority list 😅). But it could be that simply namespacing .flaglist inside geom_flag solves your immediate problem! I'll try to put some time aside soon to give it a whirl (or I would accept a PR to implement it if you have the time sooner!).

Thanks very much for reporting this (and with so much detail)!

jimjam-slam commented 1 year ago

Actually, reading the usethis::use_data() documentation, it seems like renaming lflags.rda to sysdata.lda would be a better option—I think then it could be exported on its own?

rempsyc commented 1 year ago

Thanks for the quick answer!

It seems like the lflags solution isn't working so far.

data(lflags)
#> Warning in data(lflags): data set 'lflags' not found

data(ggflags::lflags)
#> Warning in data(ggflags::lflags): data set 'ggflags::lflags' not found

library(ggflags)
#> Loading required package: ggplot2
#> Warning: package 'ggplot2' was built under R version 4.3.1

data(lflags)

lflags
#> Error in eval(expr, envir, enclos): object 'lflags' not found

ggflags::lflags
#> Error: 'lflags' is not an exported object from 'namespace:ggflags'

data(lflags, "ggflags")
#> Warning in data(lflags, "ggflags"): data set 'ggflags' not found

Created on 2023-09-10 with reprex v2.0.2

However, I have experience including data sets within packages, so perhaps I can help here. The package that I am developing that relies on ggflags is called pubmedDashboard: https://rempsyc.github.io/pubmedDashboard

The package includes four documented datasets, like a list of the US states: https://rempsyc.github.io/pubmedDashboard/reference/us_states.html

To be able to reference a package dataset, it must be documented, like here for the US states one: https://github.com/rempsyc/pubmedDashboard/blob/HEAD/R/us_states.R. The file lives in the R folder like other functions.

Since you have no documented dataset in the R folder, I think you do not need to rename the dataset, just copy (for example) my file and replace the relevant info. I can do a PR if you want.

jimjam-slam commented 1 year ago

Thanks! Maybe also give data(lflags, "ggflags") a try—data() takes a second argument called package.

But yes, I think documenting the dataset makes more sense in the long run. The package was originally designed as a playground/example before I took it over, and I think this is an aspect that could be brought up to publication standard. If you can do a PR I'd really appreciate it! I do think that switching to sysdata.rda and having it follow export rules might also be required, but see how you go just providing some stub documentation (I can fill the details in)!

rempsyc commented 1 year ago

Thanks, I've updated the previous reprex with your suggestion. I do not think using sysdata.rda is required, and it is not even clear whether this is the recommended practice—notice that this is not the default of the internal argument of use_data() (defaults to FALSE). In fact, it seems like internal = TRUE is when the dataset is only used for internal use and not to be made available to users, whereas, we want users to be able to access it—I think? I'll start the PR