Skip to content

Feature/phi imf efficiency#778

Open
daymontas1 wants to merge 14 commits into
pik-piam:masterfrom
daymontas1:feature/phi-imf-efficiency
Open

Feature/phi imf efficiency#778
daymontas1 wants to merge 14 commits into
pik-piam:masterfrom
daymontas1:feature/phi-imf-efficiency

Conversation

@daymontas1
Copy link
Copy Markdown

This PR introduces the input data for the new realization investment inefficiencies in the macro module (Available here: remindmodel/remind#2278).

Comment thread R/calcCostOfCapital.R Outdated
return(
list(
x = output, unit = "%",
description = "add some descriptive text here",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fill in a proper description, no placeholder text.

Comment thread R/calcCostOfCapital.R
@@ -0,0 +1,10 @@
calcCostOfCapital <- function() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some minimal documentation to all the calc and read functions. Minimum should be author, one short sentence what is read in/calculated, and documentation of all parameters passed to the function (if any, you don't have any).

Comment thread R/readIMF_PHI.R Outdated
readIMF_PHI <- function() {

library(dplyr)
library(readxl)
Copy link
Copy Markdown
Contributor

@fbenke-pik fbenke-pik Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use library commands within your function. these two can deleted without replacement. to specify functions, either use readxl::ready_xlsx (like you did below), or via @importFrom readxl read_xlsx on top of the function (only recommended when you use a function a lot and need to shorten code)

Comment thread R/readETH_WACC.R Outdated
@@ -0,0 +1,48 @@
#' This function reads WACC markup data across technologies for each of the REMIND regions
#library(dplyr)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove all the comments that are just commented out code and not actual documentation in this file and all the other files.

@fbenke-pik
Copy link
Copy Markdown
Contributor

First steps:

  • move latest master branch into you branch and resolve conflicts
  • run lucode2::buildLibrary() to increment library version (choose option 3). If this does not work right away, fix all the problems shown.

Comment thread R/readIMF_PHI.R Outdated
# ------------------------------------------------------------
raw_clean <- raw %>%
mutate(
Efficiency = as.numeric(Efficiency),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference columns in dplyr functions as follows
Efficiency = as.numeric(.data$Efficiency)

(same goes for Remind_Region and GDP further down below)

Comment thread R/readIMF_PHI.R Outdated
Efficiency = as.numeric(Efficiency),
GDP = as.numeric(gsub(",", "", GDP))
) %>%
filter(
Copy link
Copy Markdown
Contributor

@fbenke-pik fbenke-pik Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you filtering NA's? your source does not have any.

Comment thread R/readETH_WACC.R Outdated

# Reorder columns to: t, reg, tewacc, value
data_expanded <- data_expanded %>%
select(t, reg, tewacc, value)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to please the linter, write select("t", "reg", "tewacc", "value")

Copy link
Copy Markdown
Member

@LaviniaBaumstark LaviniaBaumstark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this PR is o.k., it can be merged independently of the REMIND-PR. Additional inptu data files can behandled (are ignored) by our input data workflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants