73 climate matching function

[SanderDevisscher]

Fixes #73
Fixes #74

I've created a new function for the Trias package which uses GBIF observations to create a series of rudimentary climate matching outputs.

To use this function 3 datapackages (observed, future & legend) have been added. These contain the shapes for the observed and future climate zones based on the Koppen Geiger classifications and their respective legends. These shapes originate from Rubel & Kottek 2010 and Beck et al 2018.

The flowchart below depicts the steps taken to create the outputs:

As extra functionality 3 extra outputs were added, namely a current climate map, future climate maps and single species maps

[SanderDevisscher]

@damianooldoni I've added an example, switched the default state of the rerun to "no" and allowed the use of a custom region shape. Besides the warnings & errors from the check below I think (and hope) the function is now ready for your review.

== Warnings ====================================================================
-- Warning (test-output_gbif_has_distribution.R:4:3): gbif_has_distribution with user parameters --
`distinct_()` was deprecated in dplyr 0.7.0.
Please use `distinct()` instead.
See vignette('programming') for more help
This warning is displayed once every 8 hours.
Call `lifecycle::last_warnings()` to see where this warning was generated.
Backtrace:
  1. testthat::expect_true(gbif_has_distribution(140563025, country = "BE")) test-output_gbif_has_distribution.R:4:2
 16. dplyr::distinct_(., .dots = names(user_properties))
 17. dplyr:::lazy_deprec("distinct")

== Failed tests ================================================================
-- Error (test-update_download_list.R:17:3): gbif download added correctly to list GBIF downloads and other status updated --
Error: unused argument (lazy = FALSE)

[ FAIL 1 | WARN 1 | SKIP 0 | PASS 811 ]
Error: Test failures
Execution halted

1 error x | 1 warning x | 3 notes x
Error: R CMD check found ERRORs
Execution halted

Exited with status 1.

[SanderDevisscher]

@damianooldoni I'm trying to implement #78 but sofar I'm having little luck. Since this is an extra feature I think it is a good idea to merge the current "working" state of the function into the master.

[damianooldoni]

Sorry @SanderDevisscher for this late answer. I have just solved the small bug behind the automatic test failures. I was also checking your function, but I stopped: I was stuck as I could not find anymore which curly bracket I erroneously deleted etc.

You can merge this PR if you need to, but these points should be tackled sooner or later:

1. GBIF instead of gbif in documentation
2. taxon_key instead of taxonkey, zip_file instead of zipfile etc.: ALL functions in this package use snake_case. I don't find any reason to have an exception for this function.
3. This function is way too long. Splitting in subfunctions will help readibility and debugging: future-you, the maintainer (that's me 😮 ) and anyone else trying to understand the workflow will be grateful. Notice that these subfunctions can be written in the same R file, just append them below the "main" function. For example, the leaflet part is clearly something you can put in a subfunction, isn't?
4. Unit-testing are needed: I can do it, but first 3 needs to be solved.
5. About the entire workflow behind this function, I really suggest you to write a vignettte about. I mean, it's a pity the schema you drew in 73 climate matching function #77 (comment) would be "lost" in this PR.

So, if you need, I approve this PR: this is what you prefer, if I understood your comment above correctly. I will then translate all my thoughts in some issues. Otherwise, I will officially "request changes" and you can work further on this branch. Let me know!

Anyway, I really appreciate your work and what this function does! 💪 A great function, an added value for this package. It's now a question of refactoring and polishing.

[SanderDevisscher]

Sorry @SanderDevisscher for this late answer. I have just solved the small bug behind the automatic test failures. I was also checking your function, but I stopped: I was stuck as I could not find anymore which curly bracket I erroneously deleted etc.

Just glad we can make some progress

You can merge this PR if you need to, but these points should be tackled sooner or later:

No specific need. But letting the branches diverge has proven a recipe for conflicts. Also since I'm very busy atm I think its best to merge this branch after I tackled remarks 1 & 2 and to work with issues for the rest.

1. GBIF instead of gbif in documentation
2. taxon_key instead of taxonkey, zip_file instead of zipfile etc.: ALL functions in this package use snake_case. I don't find any reason to have an exception for this function.

I'll fix the issues above before the merge. Those below should be turned into issues.

3. This function is way too long. Splitting in subfunctions will help readibility and debugging: future-you, the maintainer (that's me 😮 ) and anyone else trying to understand the workflow will be grateful. Notice that these subfunctions can be written in the same R file, just append them below the "main" function. For example, the leaflet part is clearly something you can put in a subfunction, isn't?

I agree I just don't exactly know how besides this is a huge work for which I do not have time atm.

4. Unit-testing are needed: I can do it, but first 3 needs to be solved.

I'll do this if you show me how (so I can do it for future functions I write)

5. About the entire workflow behind this function, I really suggest you to write a vignettte about. I mean, it's a pity the schema you drew in #77 (comment) would be "lost" in this PR.

Again I agree but do not know how to make a vignette

So, if you need, I approve this PR: this is what you prefer, if I understood your comment above correctly. I will then translate all my thoughts in some issues. Otherwise, I will officially "request changes" and you can work further on this branch. Let me know!

Anyway, I really appreciate your work and what this function does! 💪 A great function, an added value for this package. It's now a question of refactoring and polishing.

[damianooldoni]

Thanks @SanderDevisscher. I agree 100% with your answer. I will be very glad to provide you with the help you need for both unit tests and vignettes. Maybe a kind of "sprint" day in a remote place without being by nobody and any other request, will be the most effective way for this.

About snake case, be careful: not easy to change all of them without making an error. I speak about my personal experience. But you know your code better than me, of course, and so, it will work. I will now bump the version if still not done. So, please, pull from remote within 5 minutes.

[SanderDevisscher]

@damianooldoni I fixed issues 1 & 2 (partially, I'll implement more snake_case when I split the function).
Can you approve the PR and merge ?

[damianooldoni]

Nice work. 💪