search

JRaviLab / MolEvolvR

An R Package for characterizing proteins using molecular evolution and phylogeny
https://jravilab.github.io/MolEvolvR/
Other
6 stars 16 forks source link

R-CMD Check: no visible binding for global variable #4

Open the-mayer opened 2 months ago

the-mayer commented 2 months ago

Overview

Be explicit in all function calls. Utilize rlang::.data as appropriate, checking functions off the list as they have been updated.

Function List

jananiravi commented 1 month ago

@SunSummoner, this might be a good place to start. @the-mayer has started working on this for now.

SunSummoner commented 1 month ago

@jananiravi @the-mayer I will start with this. Thanks!

valentina-buoro commented 1 month ago

Hi @jananiravi , can I work on this?

jananiravi commented 1 month ago

yes, please! coordinate with @SunSummoner via github/slack to see if she's started some work already.

SunSummoner commented 1 month ago

@jananiravi I talked with @valentina-buoro and we were thinking of dividing some of the functions after getting the PR reviewed for changes.

the-mayer commented 1 month ago

Splitting the remainder (roughly) by function(s), so that multiple people may participate!

Note, most of these errors come from how R handles Non Standard Evaluation. Resolving these issues does not actually change code functionality. It will however, clear a R CMD warning, which will be important for when MolEvolvR is submitted for review. Consider the following example:

filter_function <- function(data, filter_value) {
  data %>%
    filter(numeric_column_in_data < filter_value)
  }

R knows that numeric_column_in_data can be found within the data object. But, if this code were to be written in an R package, when running devtools:check() a warning would be produced:

filter_function: no visible binding for global variable 'numeric_column_in_data'

rlang, the package that helps with NSE offers a way to deal with this, by making the function code (within a package) more explicit. By refactoring to include a .data prefix, we can clear the warning:

filter_function <- function(data, filter_value) {
  data %>%
    filter(.data$numeric_column_in_data < filter_value)
  }

Please take the comments that follow, convert to an issue, then follow the contributing guidelines and create a new branch containing your changes. Commit your work, and submit a PR when ready for review.

the-mayer commented 1 month ago

Note, in addition to undefined global variables, this function will need to declare an import for midpoint

the-mayer commented 1 month ago
the-mayer commented 1 month ago
the-mayer commented 1 month ago
the-mayer commented 1 month ago
the-mayer commented 1 month ago
the-mayer commented 1 month ago
the-mayer commented 1 month ago
valentina-buoro commented 1 month ago

Splitting the remainder (roughly) by function(s), so that multiple people may participate!

Note, most of these errors come from how R handles Non Standard Evaluation. Resolving these issues does not actually change code functionality. It will however, clear a R CMD warning, which will be important for when MolEvolvR is submitted for review. Consider the following example:

filter_function <- function(data, filter_value) {
  data %>%
    filter(numeric_column_in_data < filter_value)
  }

R knows that numeric_column_in_data can be found within the data object. But, if this code were to be written in an R package, when running devtools:check() a warning would be produced:

filter_function: no visible binding for global variable 'numeric_column_in_data'

rlang, the package that helps with NSE offers a way to deal with this, by making the function code (within a package) more explicit. By refactoring to include a .data prefix, we can clear the warning:

filter_function <- function(data, filter_value) {
  data %>%
    filter(.data$numeric_column_in_data < filter_value)
  }

Please take the comments that follow, convert to an issue, then follow the contributing guidelines and create a new branch containing your changes. Commit your work, and submit a PR when ready for review.

Noted @the-mayer

valentina-buoro commented 1 month ago
  • [x] lineage.neighbors.plot: no visible binding for global variable ‘lincount’
  • [x] lineage_sunburst: no visible global function definition for ‘d3_nest’
  • [x] lineage_sunburst: no visible binding for global variable ‘cpcols’
  • [x] lineage_sunburst: no visible global function definition for ‘sund2b’
  • [x] make_accnums_unique: no visible binding for global variable ‘accnum’
  • [x] make_accnums_unique: no visible binding for global variable ‘suffix’
  • [x] make_accnums_unique: no visible binding for global variable ‘accnum_adjusted’
  • [x] make_df_iprscan_domains: no visible binding for global variable ‘Analysis’
  • [x] make_df_iprscan_domains: no visible binding for global variable ‘AccNum’
  • [x] make_df_iprscan_domains: no visible binding for global variable ‘StartLoc’
  • [x] make_df_iprscan_domains: no visible binding for global variable ‘StopLoc’
  • [x] make_df_iprscan_domains: no visible binding for global variable ‘id_domain’

Hi fellow outreachy interns. These set of functions have been worked on.

Klangina commented 3 weeks ago

Can I work on https://github.com/JRaviLab/MolEvolvR/issues/4#issuecomment-2395224614

generate_fa2tre: no visible binding for global variable ‘tre_path’ generate_fa2tre: no visible binding for global variable ‘prot’ generate_fa2tre: no visible global function definition for ‘midpoint’

post the changes in https://github.com/JRaviLab/MolEvolvR/pull/108 ?

The warning in devtools::check post the changes are as follows:

createFA2Tree: no visible global function definition for ‘midpoint’ createFA2Tree: no visible binding for global variable ‘fit_optimized2’

@the-mayer @jananiravi