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

Prepare next release #17

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open

Prepare next release #17

wants to merge 55 commits into from

Conversation

hsonne
Copy link
Member

@hsonne hsonne commented Jan 6, 2024

This change is Reviewable

hsonne and others added 30 commits September 5, 2023 18:42
and use stats::setNames() to shorten by 1 line
so that the post-processing can remain
Start adding support for json files as input
TODO: check if I can use this more general
function to simplify or shorten the existing
approach.
Keep the name of the original list elements
in a column if there was more than one element
from which flattened data frames are joined
@hsonne hsonne requested a review from mrustl March 18, 2024 18:19
@mrustl
Copy link
Member

mrustl commented Mar 21, 2024

DESCRIPTION line 3 at r1 (raw file):

Package: kwb.umberto
Title: R package supporting UMERTO LCA at KWB
Version: 0.2.0.9000

Please bump version number before moving to "master". In addition also update NEWS.md if not already done!

@mrustl
Copy link
Member

mrustl commented Mar 21, 2024

R/import_json_files_to_excel.R line 15 at r1 (raw file):

#'   possible combinations of values in these key columns are then given in each
#'   sheet even though there are no values for these key value combinations.
#'   Default: \code{c("indicator", "process", "place", "exchange")}

does not fit with number of values for "expand_keys" in function

@mrustl
Copy link
Member

mrustl commented Mar 21, 2024

R/import_rawdata_json.R line 5 at r1 (raw file):

  #remotes::install_github("kwb-r/kwb.umberto@dev")
  
  json_dir <- "~/../Downloads/S/support/fabian/R-Umberto/Umberto11"

Add this part as new "vignette" for new UMBERTO version

Copy link
Member

@mrustl mrustl left a comment

Choose a reason for hiding this comment

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

Reviewed 14 of 16 files at r1, all commit messages.
Reviewable status: 14 of 16 files reviewed, 3 unresolved discussions (waiting on @hsonne)

Copy link
Member

@mrustl mrustl left a comment

Choose a reason for hiding this comment

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

:lgtm: minor comments for improvement ;-)

Reviewable status: 14 of 16 files reviewed, 3 unresolved discussions (waiting on @hsonne)

Copy link
Member

@mrustl mrustl left a comment

Choose a reason for hiding this comment

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

only minor comments for improvement ;-)

Reviewed 2 of 16 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @hsonne)

@mrustl
Copy link
Member

mrustl commented Mar 21, 2024

  • Add new vignette for new UMBERTO import
  • Bump version of R package for release and update NEWS.md

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.

2 participants