Update climate_match.R

[soriadelva]

fixes #110

Function converted to work on sf package instead of sp package. World maps imported via package rnaturalworld instead of rworldwap. Possible issue: this package may consider continents a little differently (e.g., Greenland is not a part of Europe while in rworldmap it was considered a part of Europe)

[damianooldoni]

Thanks @soriadelva!

I would do as follows:

1. when done, transform this draft to a Pull Request (PR).
2. @SanderDevisscher: please, review the functionality as you are the best person to do that, I think.
3. I will do a more software related review, without really going to check the functionality.

[SanderDevisscher]

trias/R/climate_match.R

Line 64 in 7cd33db

#' require('rgdal')

rgdal & rworldmap are no longer required/used. They should be removed from documentation.

[SanderDevisscher]

also add dplyr:: to L286 case_when & L296 n()

[SanderDevisscher]

Currently some function outputs have nonsensical columns. For example unfiltered has a geometry column containing point geometries while this dataframe exists of 1 row per species & climate zone combination. I would drop the geometry column using sf::st_drop_geometry(). Alternatively we can mutate the column to contain the polygon geometries of the climate zone.

Also the case for cm & filtered.

[SanderDevisscher]

devtools::check() yields the following warnings & notes

checking dependencies in R code ... WARNING
  '::' or ':::' import not declared from: 'rnaturalearth'
  Namespaces in Imports field not imported from:
    'methods' 'raster' 'rworldmap' 'sp'
    All declared Imports should be used.

❯ checking package dependencies ... NOTE
  Imports includes 25 non-default packages.
  Importing from so many packages makes the package vulnerable to any of
  them becoming unavailable.  Move as many as possible to Suggests and
  use conditionally.

❯ checking top-level files ... NOTE
  Non-standard file/directory found at top level:
    '0073402-240229165702484.zip'

❯ checking R code for possible problems ... NOTE
  apply_decision_rules: no visible binding for global variable 'n'
  apply_decision_rules: no visible binding for global variable
    'em_status'
  apply_gam: no visible binding for global variable 'em_status'
  apply_gam: no visible binding for global variable 'growth'
  apply_gam: no visible binding for global variable 'method'
  apply_gam: no visible binding for global variable 'smooth'
  apply_gam: no visible binding for global variable 'var'
  apply_gam: no visible binding for global variable 'data'
  apply_gam: no visible binding for global variable 'derivative'
  apply_gam: no visible binding for global variable 'se'
  apply_gam: no visible binding for global variable 'crit'
  apply_gam: no visible binding for global variable 'lower'
  apply_gam: no visible binding for global variable 'upper'
  climate_match: no visible binding for global variable 'legends'
  climate_match: no visible binding for global variable 'observed'
  climate_match: no visible binding for global variable 'future'
  climate_match: no visible binding for global variable 'GRIDCODE2'
  climate_match: no visible binding for global variable 'n_obs'
  climate_match: no visible binding for global variable
    'acceptedTaxonKey'
  gbif_has_distribution: no visible global function definition for
    'everything'
  get_cred: no visible global function definition for 'setNames'
  get_table_pathways: no visible binding for global variable 'n'
  indicator_introduction_year: no visible binding for global variable
    'key'
  indicator_total_year: no visible binding for global variable 'key'
  indicator_total_year: no visible binding for global variable 'year'
  verify_taxa: no visible binding for global variable '.'
  Undefined global functions or variables:
    . GRIDCODE2 acceptedTaxonKey crit data derivative em_status
    everything future growth key legends lower method n n_obs observed se
    setNames smooth upper var year
  Consider adding
    importFrom("stats", "setNames", "smooth", "var")
    importFrom("utils", "data")
  to your NAMESPACE file.

PS I also did roxygen2::roxygenise()

[SanderDevisscher]

@soriadelva I did my review of the current state if you can take a look at the devtool notes, do a devtools::check() yourself and fix any remaining (hopefully no errors,) warnings & notes and change the spatial export to a sf object I think we can merge this PR.

A suggestion for future pimping of maps would be adding the region as a polyline to all the maps and setting the default zoom to the boundingbox of the region.

[soriadelva]

@damianooldoni, when I run the devtools::check() command I receive the following warning, would you know what to do with it?:

`❯ checking data for ASCII and uncompressed saves ... WARNING

Note: significantly better compression could be obtained
      by using R CMD build --resave-data
             old_size new_size compress
future.rda      1.6Mb    852Kb    bzip2
observed.rda    1.6Mb    857Kb       xz

`

[damianooldoni]

Thanks @soriadelva. About the uncompressed savings issue, try to save again the .rda files using this syntax?

save(your_r_object,file="your_rda_file.rda",compress="xz")

[soriadelva]

save(your_r_object,file="your_rda_file.rda",compress="xz")

This works, grazie!

[damianooldoni]

You are welcome. I see a lot of notes. Most of them are typically due to the fact that while developing a package, you cannot just use df %>% dplyr::select(col1, col2) but df %>% dplyr::select("col1", "col2"), same holds true for dplyr::rename() and dplyr::relocate(), i.e. all functions using tidyselect under the hood.
All other dplyr functions, like dplyr::filter() should use the .data$ prefix, e.g. df %>% dplyr::filter(.data$col1 > 0) or df %>% dplyr::mutate(newcol = .data$col1 +1) or df %>% dplyr::group_by(.data$col1).

Can you do a screening on climate_match.R first? Then I will review and I will push some other changes if I see I can solve some of the notes.

In any case, we are near to merge 🥳

[soriadelva]

You are welcome. I see a lot of notes. Most of them are typically due to the fact that while developing a package, you cannot just use df %>% dplyr::select(col1, col2) but df %>% dplyr::select("col1", "col2"), same holds true for dplyr::rename() and dplyr::relocate(), i.e. all functions using tidyselect under the hood. All other dplyr functions, like dplyr::filter() should use the .data$ prefix, e.g. df %>% dplyr::filter(.data$col1 > 0) or df %>% dplyr::mutate(newcol = .data$col1 +1) or df %>% dplyr::group_by(.data$col1).

Can you do a screening on climate_match.R first? Then I will review and I will push some other changes if I see I can solve some of the notes.

In any case, we are near to merge 🥳

@damianooldoni, I fixed all notes except those that are related to the .rda files in the /data folder. Example:
climate_match: no visible binding for global variable 'legends'. legends is successfully loaded with devtools::load_all() and the documentation under the /R folder is all in place, but still devtools::check() does not seem to recognize this dataset properly. The same problem exists for the other .rda files in that folder. Would you know how to solve this? This question has already been asked on Stackoverflow (I checked the one response, but the problem does not seem to be the documentation): https://stackoverflow.com/questions/64195806/rnote-no-visible-binding-for-global-variable-after-devtoolsuse-data

[soriadelva]

@damianooldoni, okay to merge? :)

[SanderDevisscher]

ok for me

[damianooldoni]

Thanks @soriadelva. I will try my best to check all of this during the afternoon.

[soriadelva]

Thanks @soriadelva. I will try my best to check all of this during the afternoon.

Thanks @damianooldoni ! and no worries, you'll see when you have time!

[damianooldoni]

Sorry, @soriadelva. I was very busy with the preparation of the coding club of today. I will try to do it this evening.

[soriadelva]

Sorry, @soriadelva. I was very busy with the preparation of the coding club of today. I will try to do it this evening.

@damianooldoni no worries at all (sorry, I didn't mean to hurry you up!).

[damianooldoni]

I didn't solved the global variable undefined for the three built-in data objects used in climate_match function. But I removed some other similar notes and updated the versions of actions in the github workflows.
And the most important stuff: I added @soriadelva as author of the package 👏 congrats!

Let's merge now.

[soriadelva]

I didn't solved the global variable undefined for the three built-in data objects used in climate_match function. But I removed some other similar notes and updated the versions of actions in the github workflows. And the most important stuff: I added @soriadelva as author of the package 👏 congrats!

Let's merge now.

Thanks @damianooldoni 😃