Skip to content

Commit

Permalink
small adjustments to export_gtfs() tests, code style and comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dhersz committed May 25, 2021
1 parent 6ac9ab8 commit f2ad18a
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 20 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Type: Package
Package: gtfsio
Title: Read and Write General Transit Feed Specification (GTFS) Files
Version: 0.1.0.9003
Version: 0.1.0.9004
Authors@R:
c(person(given = "Daniel",
family = "Herszenhut",
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

## Bug fixes

- Fixed a bug in `export_gtfs()` when `as_dir` was set to `TRUE` and `path` was set to `tempdir()` in #15. The function now returns an (intentional) error if `tempdir()` is passed to `path`. Thanks Flavio Poletti (@polettif).

## New features

- `import_gtfs()` has a new `skip` parameter. It may be used similarly to `files`, but you specify the files you *don't want* to read, instead of the ones you do. Thanks Flavio Poletti (@polettif).

## Notes

# gtfsio 0.1.0
Expand Down
28 changes: 17 additions & 11 deletions R/export_gtfs.R
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ export_gtfs <- function(gtfs,
stop("'path' must be a string (a character vector of length 1).")

if (path == tempdir())
stop("Please use 'path = tempfile()' instead of tempdir() to designate temporary directories")
stop(
paste0(
"Please use 'tempfile()' instead of 'tempdir()' to designate ",
"temporary directories."
)
)

if (!is.null(files) & !is.character(files))
stop("'files' must either be a character vector or NULL.")
Expand Down Expand Up @@ -98,7 +103,7 @@ export_gtfs <- function(gtfs,
)

if (as_dir & grepl("\\.zip$", path)) {
stop("path cannot have '.zip' extension with as_dir=TRUE")
stop("'path' cannot have '.zip' extension when 'as_dir' is TRUE.")
}

extra_files <- setdiff(files, names(gtfs_standards))
Expand Down Expand Up @@ -137,16 +142,16 @@ export_gtfs <- function(gtfs,
paste0("'", missing_files, "'", collapse = ", ")
)

# use path or create temp directory where files should be written to
if (as_dir) {
# write files either to a temporary directory (if as_dir = FALSE), or to path
# (if as_dir = TRUE)

if (as_dir)
tmpd <- path
unlink(tmpd, recursive = TRUE)
dir.create(tmpd)
} else {
else
tmpd <- tempfile(pattern = "gtfsio")
unlink(tmpd, recursive = TRUE)
dir.create(tmpd)
}

unlink(tmpd, recursive = TRUE)
dir.create(tmpd)

# write files to 'tmpd'

Expand Down Expand Up @@ -177,14 +182,15 @@ export_gtfs <- function(gtfs,

}

# write result to 'path'
# zip the contents of 'tmpd' to 'path', if as_dir = FALSE
# remove the file/directory in 'path' (an error would already have been thrown
# if 'path' pointed to an existing file that should not be overwritten).
# this action prevents zip::zip() from crashing R when 'path' exists, but is a
# directory, not a file
# related issue: https://github.com/r-lib/zip/issues/76

if (!as_dir) {

unlink(path, recursive = TRUE)

filepaths <- file.path(tmpd, paste0(files, ".txt"))
Expand Down
2 changes: 1 addition & 1 deletion R/new_gtfs.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#' @examples
#' gtfs_path <- system.file("extdata/ggl_gtfs.zip", package = "gtfsio")
#'
#' tmpdir <- file.path(tempfile(), "new_gtfs_example")
#' tmpdir <- tempfile(pattern = "new_gtfs_example")
#' zip::unzip(gtfs_path, exdir = tmpdir)
#'
#' agency <- data.table::fread(file.path(tmpdir, "agency.txt"))
Expand Down
4 changes: 2 additions & 2 deletions codemeta.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"codeRepository": "https://github.com/r-transit/gtfsio",
"issueTracker": "https://github.com/r-transit/gtfsio/issues",
"license": "https://spdx.org/licenses/MIT",
"version": "0.1.0.9003",
"version": "0.1.0.9004",
"programmingLanguage": {
"@type": "ComputerLanguage",
"name": "R",
Expand Down Expand Up @@ -123,7 +123,7 @@
"sameAs": "https://CRAN.R-project.org/package=zip"
}
],
"fileSize": "128.175KB",
"fileSize": "128.882KB",
"keywords": [
"r",
"gtfs"
Expand Down
21 changes: 17 additions & 4 deletions inst/tinytest/test_export_gtfs.R
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,23 @@ expect_error(
)
)

# raise an error if path has a '.zip' extension but 'as_dir' is TRUE

expect_error(
export_gtfs(gtfs, tmpf, as_dir = TRUE),
pattern = "'path' cannot have '\\.zip' extension\\ when 'as_dir' is TRUE\\."
)

# raise an error if 'as_dir' is TRUE and 'path' is tempdir()

expect_error(
export_gtfs(gtfs, tempdir(), as_dir = TRUE),
pattern = paste0(
"Please use 'tempfile\\(\\)' instead of 'tempdir\\(\\)' to designate ",
"temporary directories."
)
)

# object should be exported as a zip file by default

export_gtfs(gtfs, tmpf)
Expand Down Expand Up @@ -241,14 +258,10 @@ expect_true(any(grepl("^GTFS object successfully zipped to ", out)))

# as_dir = TRUE

expect_error(export_gtfs(gtfs, tempdir(), as_dir = TRUE),
"Please use 'path = tempfile\\(\\)' instead of tempdir\\(\\) to designate temporary directories")

expect_message(export_gtfs(gtfs, tmpd, as_dir = TRUE, quiet = FALSE))
out <- capture.output(
export_gtfs(gtfs, tmpd, as_dir = TRUE, quiet = FALSE),
type = "message"
)
expect_true(any(grepl("^Writing text files to ", out)))
expect_true(any(grepl("^ - Writing ", out)))
expect_true(any(grepl("^Writing text files to ", out)))
2 changes: 1 addition & 1 deletion man/new_gtfs.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f2ad18a

Please sign in to comment.