forestgeo / agbfluxes Goto Github PK

From @maurolepore on September 18, 2018 14:42

About your DESCRIPTION file:

• Type: I'm not sure what this field does.
• Version: Consider using a development version until released. The book R Packages recommends adding .9000 to the end of a development verison number to clearly indicate it is not a released version.
• Authorand Maintainer: Both fields can be documented with person()`. For an example see: https://github.com/forestgeo/fgeo/blob/master/DESCRIPTION

In general

The easiest way to create a good DESCRIPTION file is by using the one you get when you create a new package with usethis::create_package(), then add infrastructure as you need it (e.g. usethis::use_gpl3_license().

Copied from original issue: ErvanCH/AGBflux_pack#4

Hey Mauro,
I have a couple of changes to the package (remove stuff and update scripts). I'd like now to rebuild the package to get the ints/extdata/data folder updated and use it to update the readme.
However, after having passed the check, I can install and build the package. It says that "data.table" is missing... (see below). Any clue?

I flagged a few things with TODO and FIXME that need to be addressed.

From: Helene Muller-Landau
Sent: Friday, October 12, 2018 10:07:01 AM (UTC-05:00) Eastern Time (US & Canada)
To: KC Cushman; Ervan Rutishauser; Lepore, Mauro
Subject: biomass R package for ForestGeo plots

KC is analyzing taper for multiple plots and testing alternative methods to handle changes in points of measurement. Ervan and Mauro are working on an R package for biomass stock and flux estimates at ForestGEO plots, including the taper correction as part of that. Ervan also mentioned that in working with Mauro on that, Mauro told him about an R function (regression test if I remember the name?) that can help in assessing how code changes propagate to output changes, which was itself useful.

KC, I suggest you share a draft of the manuscript with Ervan and Mauro. And perhaps the three of you, or some subset, should chat about whether some of this could or should be incorporated into the R package.

What are your main functions? There should be one .R file per main function and its helpers.

• Document the souce of each raw dataset and how to go from raw to clean data in a file in data-raw/data.R.
• Documet details of each dataset in R/data.R (or R/dataset1.R, R/dataset2.R, etc.).

From @maurolepore on September 18, 2018 15:23

• Once you are using automated unit tests, you may want to measuere code cover. For example with the covr package (see devtools::use_coverage()).

• Once you are confident of the checks you do locally, you may want to use continuous integration. For example with TravisCI (see devtools::use_travis()).

Copied from original issue: ErvanCH/AGBflux_pack#14

From @maurolepore on September 18, 2018 15:8

Running devtools::check() should warn you if the documentation of any function is incomplete (find more details here; http://r-pkgs.had.co.nz/check.html).

One good way to not miss any important piece of documentation is to step on the function's name and press Ctrl + Shift + Alt + R. That will automatically write the roxygen skeleton of key documentation inluding all parameters. You can also Insert Roxygen Skeleton form the Code menu:

Copied from original issue: ErvanCH/AGBflux_pack#8

RE: On Thu, Oct 4, 2018 at 8:22 AM Ervan Rutishauser [email protected] wrote:

The main function works fine on my end. Could you test and let me know, please?

Unfortunately it fails on my end. I tried from the AGBfluxes package itself, and from a new project with just the data (image below). Can you see what I'm doing wrong?

Please briefly describe your problem and what output you expect. If you have a question, please don't use this form. Instead, ask on https://stackoverflow.com/, https://community.rstudio.com/, https://github.com/forestgeo/forum/ or email Mauro Lepore at [email protected].

Please include a minimal reproducible example (AKA a reprex). If you've never heard of a reprex before, start by reading https://www.tidyverse.org/help/#reprex.

Brief description of the problem

# insert reprex here

From @maurolepore on September 18, 2018 14:51

A package website can host any metadata associated to your package including documentation showing how to use your package. For example: https://forestgeo.github.io/fgeo/.

You can build (and update) a package website by running pkgdown::build_site(). The website will live in the folder docs/. To host on GitHub go to: Settings > GitHubPages > Source > master branch /docs folder. This mute video shows the process: https://www.youtube.com/watch?v=QVxJyQm-pBg&index=10&list=PLvgdJdJDL-AOJKDiV6U09LdOYOTgFpQqN&t=1s

Copied from original issue: ErvanCH/AGBflux_pack#6

From @maurolepore on September 18, 2018 14:46

I believe you already created the skeleton of a readme file with usethis::use_readme_rmd(). It doesn't yet appear on GitHub, meaning that you haven't pushed it yet.

The information in README will appear at the Home of the package website, and also on the front page of your package repository. README should cover the goal of the package, installation instructions, and an example. Remember to knit that .Rmd file whenever you update it. That will generate a .md version. Don't edit the .md version; edit the .Rmd only.

Copied from original issue: ErvanCH/AGBflux_pack#5

The object DF gets saved as df, and this is a problems because it clashes with stats::df(). I renamed df to dfm, but it seems that this object isn't used. Should we remove it?

Now, data_preparation() has a lot of arguments, and many of them are compulsory. This makes it difficult for users to use because they need to type a lot. Better is to provide sensible defaults for as much as possible.

Please briefly describe your problem and what output you expect. If you have a question, please don't use this form. Instead, ask on https://stackoverflow.com/, https://community.rstudio.com/, https://github.com/forestgeo/forum/ or email Mauro Lepore at [email protected].

Please include a minimal reproducible example (AKA a reprex). If you've never heard of a reprex before, start by reading https://www.tidyverse.org/help/#reprex.

Brief description of the problem

# insert reprex here

Relates to #29 (comment)

Do you don't need to @export all these functions? If some functions are internal the code becomes more flexible because it's easier to change internal functions than exported ones. Also we can save some time because internal functions don't need very deep documentation.

From @maurolepore on September 18, 2018 15:35

One way to improve your functions with confidence that you are not breaking anything is to run regression tests -- tests that automatically compare the result of a computation against a know result. You can do this with testthat::expect_known_output(). It is a fairly advanced topic and I'm happy to help you.

Copied from original issue: ErvanCH/AGBflux_pack#16

Hi Mauro,

I tried to install the package I pushed today to get all objects and the functions to work poperly, but it fails. Here is what I got after having called:
install_github("forestgeo/AGBfluxes",force=T)

..
(lots of stufff)
..
** testing if installed package can be loaded
Error: package or namespace load failed for 'AGBfluxes' in namespaceExport(ns, exports):
undefined exports: data_correction
Error: loading failed
Execution halted
ERROR: loading failed

Any hint?

When you create a package using the RStudio GUI instead of usethis::create_package() you need to manually check this box:

Project Options > Build Tools > Generate documentation with Roxygen

From @maurolepore on September 18, 2018 15:39

Show me and other users what your package does. You'll can show some features in README (ErvanCH/AGBflux_pack#5). To show more you can use a vignette (usethis::use_vignette()). Vignettes are a great place to show more complex examples and also to show how your package relates to other packages.

Copied from original issue: ErvanCH/AGBflux_pack#17

From @maurolepore on September 18, 2018 15:18

I recommend this style guide http://style.tidyverse.org/ because it is well documented and current.

You can re-format code with Control + Shift + A. This will add spaces after commas, brake lines to fit within 80 characters width, add spaces around equal signs, etc. You may dislike that formatting style butitiseasiertoread. Also see the styler package and the function goodpractice::gp().

Some specific comments:

• Consider grouping functions in files by topic.

• Avoid using tab to insert spaces. Instead use space.

• Keep the widh of each line no longer than 80 characters. This improves readability. To achive this:

  • Set a guide line at 80 characters page width: Tools > Global Options > Code > Display > Show Margin > Margin Column = 80.

  • For comments (both normal comments and roxygen comments), master these two keyboard shortcuts:

    • Shift + Control + C to comment a line/paragraph.
    • Shift + Control + / to brake a paragraph into lines no longer than 80 characters.

Copied from original issue: ErvanCH/AGBflux_pack#11

Hi Mauro,

Thanks for your help. I am working in background to fix/update/examplify the functions. While running my code again for my paper, I got a strange behaviour and I found the problem.
In function "computeAGB" (l, 466-481), you probably cleaned up the following expression

df['Family' == "Arecaceae", "agb" := agbPalm(dbh2)]
by
df["Family" == "Arecaceae", "agb" := agbPalm(dbh2)]

However, data.table doesn't know how to deal with " " and fails to do the job (and messed up my whole analysis).
Shall I kindly ask you to document all changes that you are doing for me to double check, please.
Thanks
Ervan

From @maurolepore on September 18, 2018 15:12

You are already testing, but likely in a non-systematic way: you write a function, run it, and see its output on the console to check check if it's right. You will save you a lot of time if you test systematically, with an automatic unit testing system such as the testthat package. To use testthat run:

usethis::use_testthat()

Then you will see tests/ in the root directory of your package. Then you add new tests with:

usethis::use_test("name-of-the-file-containing-the-functions-you-want-to-test")

This video shows the testing workflow: https://youtu.be/Ajtfdc2hnxo

Copied from original issue: ErvanCH/AGBflux_pack#10

From @maurolepore on September 18, 2018 15:12

You are already testing, but likely in a non-systematic way: you write a function, run it, and see its output on the console to check check if it's right. You will save you a lot of time if you test systematically, with an automatic unit testing system such as the testthat package. To use testthat run:

usethis::use_testthat()

Then you will see tests/ in the root directory of your package. Then you add new tests with:

usethis::use_test("name-of-the-file-containing-the-functions-you-want-to-test")

This video shows the testing workflow: https://youtu.be/Ajtfdc2hnxo

Copied from original issue: ErvanCH/AGBflux_pack#10

The code should be profiled, but these code chunks are likely slow: here and here.

Unless you are using a lot of funcitons from a package, it's best to import specific functions instead of all of them.

GOOD

#' @importFrom BIOMASS function1 function2 ...

BAD

#' @import BIOMASS

From @maurolepore on September 18, 2018 15:20

If the author of the package is the author of a function, then that author needs not to be explicitely mentioned in each help file. In other words, use @author in help files of functions written but people other than the authors of the package.

Copied from original issue: ErvanCH/AGBflux_pack#12

This is a common refactoring, and seems pretty useful for making data_preparation() easier to use.

I'll refer to this issue as I address problems identified with devtools::check().

Ervan has data that is a little different from what master has. The source of the data was Ervan, anyway (see data-raw) so adding this data manually now is no different form what it was originally.

@maurolepore , @ervanSTRI ,

I'm looking for a concise (scientific) description of exactly what this code does, but I'm not seeing it anywhere in this repo. Could you please add that info (or make it easier to find)?