jordagaman / zddr

A zero-suppressed binary decision diagram implementation for R
https://jordagaman.github.io/zddr/
Other
3 stars 0 forks source link

Namespace for zdd functions #23

Open warner-pra opened 3 years ago

warner-pra commented 3 years ago

https://github.com/jordagaman/zddr/blob/7248abe0e8eadfad14408d54cbedf631190493a4/R/zdd.R#L68

A namespace is needed for the line above (zddr::zdd_store). This is causing an error in a package I am developing that uses zddr::zdd():

Error: Problem with `mutate()` column `zobj`.
ℹ `zobj = list(zddr::zdd(eventID))`.
✖ object 'zdd_store' not found
warner-pra commented 3 years ago

These will likely cause errors as well:


zddr::as_zdd https://github.com/jordagaman/zddr/blob/7248abe0e8eadfad14408d54cbedf631190493a4/R/zdd_or.R#L20


zddr::zdd_union https://github.com/jordagaman/zddr/blob/7248abe0e8eadfad14408d54cbedf631190493a4/R/zdd_or.R#L22


zddr::as_zdd https://github.com/jordagaman/zddr/blob/7248abe0e8eadfad14408d54cbedf631190493a4/R/zdd_and.R#L18


zddr::zdd_crossproduct https://github.com/jordagaman/zddr/blob/7248abe0e8eadfad14408d54cbedf631190493a4/R/zdd_and.R#L20

warner-pra commented 3 years ago

I investigated a little more, turns out this is the same issue as was brought up in #7. I listed zddr as imports in my package, not depends. With imports I still need a separate library(zddr) call outside the package to make it work, with depends it works as expected because it attaches zddr. As explained in https://r-pkgs.org/namespace.html:

Listing a package in either Depends or Imports ensures that it’s installed when needed. The main difference is that where Imports just loads the package, Depends attaches it. There are no other differences. The rest of the advice in this chapter applies whether or not the package is in Depends or Imports.

Unless there is a good reason otherwise, you should always list packages in Imports not Depends. That’s because a good package is self-contained, and minimises changes to the global environment (including the search path). The only exception is if your package is designed to be used in conjunction with another package. For example, the analogue package builds on top of vegan. It’s not useful without vegan, so it has vegan in Depends instead of Imports. Similarly, ggplot2 should really Depend on scales, rather than Importing it.

@jordagaman: I'm okay with closing this Issue based on my workaround in my package. Just double-check the calls to zdd_store inside the package. Only Line 68 copied above is missing the zddr::, could this be causing the issue? Would be ideal to only use imports instead of depends.