Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add im- and export for IIASA xlsx files #38

Merged
merged 5 commits into from
Dec 13, 2022

Conversation

orichters
Copy link
Contributor

  • submission to IIASA database and downloads from IIASA database are in xlsx format. This PR allows to read those files into a quitte object, and to write them such that they can be submitted
  • It works, but I'm not so sure whether function names were optimally chosen (I avoided write.xlsx as function name as it already exists in xlsx package), whether tests are sufficient etc.
  • I left Michaja as "coauthor" of the function, as I basically copied much of the code from write.mif.
  • buildLibrary created lots of changes, hope they are ok…
  • improvements or corrections welcome

R/read.quitte.R Outdated Show resolved Hide resolved
R/write.IAMCxlsx.R Outdated Show resolved Hide resolved
setdiff(colnames(.), default_columns),
last(default_columns)) %>%
arrange(.data$Period) %>%
mutate(!!sym('Value') := ifelse(! is.finite(!!sym('Value')) | !!sym('Value') == "", 0, !!sym('Value'))) %>%

Choose a reason for hiding this comment

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

  1. Is that desired behaviour? Seems to me putting/leaving NA (or "N/A" or "n_a" or whatever IIASA is using) instead of valid-looking zeros would make more sense …
  2. mutate(Value = ifelse(!is.finite(.data$Value) | .data$Value == "", 0, 
                          .data$Value))
    
    is more concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Asked the IIASA admins about that. I just adopted Jeromes solution for NGFS, but maybe this is not accurate (anymore?).

Choose a reason for hiding this comment

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

In any case, an easier (and probably faster, but who is counting) way would be

-        arrange(.data$Period) %>%
-        mutate(!!sym('Value') := ifelse(! is.finite(!!sym('Value')) | !!sym('Value') == "", 0, !!sym('Value'))) %>%
-        pivot_wider(names_from = 'Period', values_from = 'Value')
+        filter(is.finite(.data$Value), '' != .data$Value) %>% 
+        pivot_wider(names_from = 'Period', names_sort = TRUE,
+                    values_from = 'Value', values_fill = 0)

If IIASA says "use 'n_a'", you will have to convert to strings first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agreed with IIASA that writing files with empty cells is best. Using NA as values_fill leads to empty cells in the xlsx, so I implemented it like that, no need to convert to string though, luckily.

.Rbuildignore Show resolved Hide resolved
R/read.quitte.R Outdated
Comment on lines 63 to 64
periods <- names(data)[grep("^[0-9]{4}$", names(data))]
data <- as.quitte(pivot_longer(data, all_of(periods), names_to = 'period', values_drop_na = drop.na))

Choose a reason for hiding this comment

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

That's quite long-winded … why not pivot_longer(matches("^[0-9]{4}$")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I copied that from this line

Choose a reason for hiding this comment

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

… that actually predates match() (and pivot_longer(), for that matter). :p

@orichters
Copy link
Contributor Author

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q: Should we also add something like this?

            missing.default.columns <- default.columns[! default.columns %in% tolower(colnames(data))]
            if (length(missing.default.columns) > 0) {
                stop(f, " misses the default columns ", paste(missing.default.columns, collapse = ", "))
            }

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member

Sure.

@orichters
Copy link
Contributor Author

Sure.

Opted for warning, I think that is sufficient. as.quitte adds the missing columns with (Missing) and so you can continue with the data.

@orichters orichters merged commit af3b955 into pik-piam:master Dec 13, 2022
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.

None yet

2 participants