search

nc-minibbs / mbbs

A repository for the Mini-Bird Breeding Survey data
https://minibbs.us
Other
2 stars 0 forks source link

Cran Checks: No Visible Binding for Global Variables #48

Open IJBG opened 7 months ago

IJBG commented 7 months ago

devtools::check() is passing with no warnings, but some notes. The longest one is about lacking visible binding for global variables. I found some suggestions for fixing it: https://www.r-bloggers.com/2019/08/no-visible-binding-for-global-variable/

@bsaul is this as issue that needs fixing? Would there be anything wrong with using the first option proposed in the above blog post, eg:

For our package, as there are literally thousands of variables to list in this file, it makes it very difficult to maintain this list and makes the file very long. If, however, the variables belong to data which are stored within your package then this can be greatly simplified to
globalVariables(names(my_data))
bsaul commented 7 months ago

The option you suggested sounds good to me

IJBG commented 6 months ago

I was going add this today, but..what file do I actually add this to? I get an error adding it to DESCRIPTION and NAMESPACE has the '# Generated by roxygen2: do not edit by hand' warning.

The lines to add are globalVariables(names(mbbs)) globalVariables(names(mbbs_survey_events)) globalVariables(names(mbbs_routes))

That might not cover all of them but it's a good place to start before hunting down smaller functions.

bsaul commented 6 months ago

Try adding to mbbs_data.R

bsaul commented 6 months ago

@IJBG - I have a few minutes, so I'm going to try to knock this out real quick.

bsaul commented 6 months ago

I like the solution of using the .data pronoun.

IJBG commented 6 months ago

error with new data$ tidyverse isn't playing nice with the .data$, still working but giving warnings.

bsaul commented 5 months ago

boo.

bsaul commented 5 months ago

I'm going to open a new issue to be resolved in new PR.

bsaul commented 5 months ago

I'm going to open a new issue to be resolved in new PR.

Nevermind. Let's just keep using this issue.