ropensci / git2rdata

An R package for storing and retrieving data.frames in git repositories.
https://ropensci.github.io/git2rdata/
GNU General Public License v3.0
99 stars 12 forks source link

finish vignette #3

Closed ThierryO closed 5 years ago

codecov-io commented 5 years ago

Codecov Report

Merging #3 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #3   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           7      7           
  Lines         321    340   +19     
=====================================
+ Hits          321    340   +19
Impacted Files Coverage Δ
R/rm_data.R 100% <ø> (ø) :arrow_up:
R/recent_commit.R 100% <ø> (ø) :arrow_up:
R/clean_data_path.R 100% <ø> (ø) :arrow_up:
R/read_vc.R 100% <100%> (ø) :arrow_up:
R/meta.R 100% <100%> (ø) :arrow_up:
R/auto_commit.R 100% <100%> (ø) :arrow_up:
R/write_vc.R 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1027df6...ddd696f. Read the comment docs.

florisvdh commented 5 years ago

Really well done, impressive! I've proposed a few vignette updates in PR #4

I could only find a small (luxury?) problem with the way variables are sorted. To be regarded as a feature request. I expect that users often put their variables in some meaningful, desired order, but this may not always coincide with their desired order of variables for sorting the rows. So for this usecase, after using read_vc(), the user would need to reorder the variables each time to get back the original order of the variables.

So perhaps it is worth considering the ability to store both the original order of the variables and the preferred order of sorting variables in the yml file separately.

ThierryO commented 5 years ago

The sorting request is solved in 4d352cd. The order of the columns at the time of writing the metadata will be used.

stijnvanhoey commented 5 years ago

Is therer any specific reason for the usage of the S4 approach using the setGeneric and setMethod pattern?

ThierryO commented 5 years ago

I like to use S4. It is more strict and rigid than S3.

ThierryO commented 5 years ago

readr is now used to read and write the plain text files. This will break old stuff, but since I'm probably the only user, it won't bug other people ;-)

Benefits:

lintr-bot commented 5 years ago

R/write_vc.R:150:61: style: Commas should never have a space before.

​    raw_data <- raw_data[do.call(order, raw_data[sorting]), , drop = FALSE]
                                                           ~^

tests/testthat/test_b_special.R:21:19: style: Commas should never have a space before.

​  ds[order(ds$a), , drop = FALSE],
                 ~^

tests/testthat/test_b_special.R:31:19: style: Commas should never have a space before.

​  ds[order(ds$a), , drop = FALSE],
                 ~^
lintr-bot commented 5 years ago

R/write_vc.R:150:61: style: Commas should never have a space before.

​    raw_data <- raw_data[do.call(order, raw_data[sorting]), , drop = FALSE]
                                                           ~^

tests/testthat/test_b_special.R:21:19: style: Commas should never have a space before.

​  ds[order(ds$a), , drop = FALSE],
                 ~^

tests/testthat/test_b_special.R:31:19: style: Commas should never have a space before.

​  ds[order(ds$a), , drop = FALSE],
                 ~^
lintr-bot commented 5 years ago

R/write_vc.R:150:61: style: Commas should never have a space before.

​    raw_data <- raw_data[do.call(order, raw_data[sorting]), , drop = FALSE]
                                                           ~^

tests/testthat/test_b_special.R:21:19: style: Commas should never have a space before.

​  ds[order(ds$a), , drop = FALSE],
                 ~^

tests/testthat/test_b_special.R:31:19: style: Commas should never have a space before.

​  ds[order(ds$a), , drop = FALSE],
                 ~^