Brandon-mcn / ghgtools

https://brandon-mcn.github.io/ghgtools/
Other
1 stars 3 forks source link

ghgtools Package Review #1 #2

Open AubreyLaPlante opened 2 months ago

AubreyLaPlante commented 2 months ago

I worked through the code, and I made comments in the vignette, which I will upload separately. The package installed fine and ran fine, but I did encounter an error when I tried to check() which you may or may not already know about.

Problems : a) During check(), one error and one note were produced The error is duplicated below: ❯ checking re-building of vignette outputs ... ERROR Error(s) in re-building vignettes: ... --- re-building ‘ghgtools.Rmd’ using rmarkdown Error: processing vignette 'ghgtools.Rmd' failed with diagnostics: The function xfun::isFALSE() will be deprecated in the future. Please consider using base::isFALSE(x) or identical(x, FALSE) instead. --- failed re-building ‘ghgtools.Rmd’

SUMMARY: processing the following file failed: ‘ghgtools.Rmd’

Error: Vignette re-building failed. Execution halted

❯ checking R code for possible problems ... NOTE create_efl: no visible binding for global variable ‘EFL’ create_efl: no visible binding for global variable ‘ar’ create_efl: no visible binding for global variable ‘kgco2e_perunit’ create_efl: no visible binding for global variable ‘ghg_perunit’ create_efl: no visible binding for global variable ‘gwp’ create_efl: no visible binding for global variable ‘ef_publishdate’ create_efl: no visible global function definition for ‘.’ create_efl: no visible binding for global variable ‘ef_source’ create_efl: no visible binding for global variable ‘ef_activeyear’ create_efl: no visible binding for global variable ‘service_type’ create_efl: no visible binding for global variable ‘unit’ create_efl: no visible binding for global variable ‘emission_category’ create_efl: no visible binding for global variable ‘service_subcategory1’ create_efl: no visible binding for global variable ‘service_subcategory2’ create_efl: no visible binding for global variable ‘emission_scope’ create_efl: no visible binding for global variable ‘country’ create_efl: no visible binding for global variable ‘subregion’ create_templates: no visible binding for global variable ‘ActivityData’ create_templates: no visible binding for global variable ‘AssetPortfolio’ ghg_inventory: no visible binding for global variable ‘Activity_Data’ ghg_inventory: no visible binding for global variable ‘Asset_Portfolio’ ghg_inventory: no visible binding for global variable ‘Ecat_lookup’ ghg_inventory: no visible binding for global variable ‘eGRIDlookup’ ghg_inventory: no visible binding for global variable ‘EF_Library’ ghg_inventory: no visible binding for global variable ‘kg_co2e’ ghg_inventory: no visible binding for global variable ‘usage’ ghg_inventory: no visible binding for global variable ‘kgco2e_perunit’ ghg_inventory: no visible binding for global variable ‘mt_co2e’ Undefined global functions or variables: . ActivityData Activity_Data AssetPortfolio Asset_Portfolio EFL EF_Library Ecat_lookup ar country eGRIDlookup ef_activeyear ef_publishdate ef_source emission_category emission_scope ghg_perunit gwp kg_co2e kgco2e_perunit mt_co2e service_subcategory1 service_subcategory2 service_type subregion unit usage Consider adding importFrom("stats", "ar") to your NAMESPACE file.

Found the following assignments to the global environment: File ‘ghgtools/R/ghg_inventory.R’: assign("Activity_Data", Activity_Data, envir = .GlobalEnv) assign("Asset_Portfolio", Asset_Portfolio, envir = .GlobalEnv) assign("EF_Library", EF_Library, envir = .GlobalEnv) assign("GHG_rawdata", GHGrawdata, envir = .GlobalEnv)

b) Building vignette worked Output created: ghgtools.html --- finished re-building ‘ghgtools.Rmd’

ℹ Copying vignettes ℹ Moving ghgtools.html and ghgtools.R to doc/ ℹ Copying ghgtools.Rmd to doc/ ℹ Building vignette index

c) The skeleton code shows me a really cool way of simply doing this (ghg) process. The introduction and explanation of each parameter (assets, asset portfolio, efl, etc) was well written. I understand what you are doing at a basic level, and when I ran the entire vignette as-is, the figures all made sense to me. d) I think a little more hand-holding for the user would be great. Some examples (where possible), more help documentation. Many of the functions feel very black-box-y such that there are no user inputs and I had to go into the .R file to look at the code to figure out what the function does. e) Overall I think this code is really cool and I would suspect that it will work for a variety of table inputs? I wasn't able to try other datasets yet but I suspect this could be used to combine datasets related to both carbon output and carbon capture (when those companies really get up and running). f) Setting the working directory doesn't change what data the code runs on. I tried to set the working directory to a new folder, with some data in it, to run the code. All of the code appears to default to the test dataset in the internal package folder, no matter what other data I try to process (located in package-external directories/working drives). I tried downloading new data and I moved your test data to a different folder and neither worked.

AubreyLaPlante commented 2 months ago
#I couldnt change the working directory that the package was referencing
setwd("~/Documents/R_package_sem/test")
create_efl("AR5")



I'm not really sure but maybe the functions that don't have user input could just be hidden, so when I push the magic 'create_efl' button, I get all these nice tables in the environment?

check_load_data()


same comments as above, where the function doesn't have inputs, and the help section is a little bare.

ghg_inventory()


The following statement might be better above the ghg_inventory function section? ghg_inventory() has merged our AssetPortoflio, ActivityData, and EFL to generate a GHG emissions inventory report. Let's take a quick look at how we might visualize this data.


library(ggplot2)

#I know its only two lines, but this could possibly be a function. 
#Otherwise, just have more comments so the users know what goes where, 
#i.e., break the code down a little more in the plotting sections

ghg_summary <- GHG_rawdata[, .(EmissionTotal = sum(mt_co2e)), by = emission_scope]
ghg_summary[, percentage := (EmissionTotal / sum(EmissionTotal))*100]
GHG_sum_chart <- ggplot(ghg_summary, aes(x = emission_scope, y = EmissionTotal)) +
  geom_bar(stat = "identity", width = 0.8, fill = "grey", color = "black") +
  geom_text(aes(label = paste0(round(percentage), "%")),
            position = position_stack(vjust = 0.5),
            size = 4,
            fontface = "bold",
            color = "black") +
  labs(title = "GHG Emissions Total",
       x = "",
       y = "MT CO2e")
print(GHG_sum_chart)