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

Return File Path after Writing in fwrite #6181

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@

14. `fread` loads `.bgz` files directly, [#5461](https://github.com/Rdatatable/data.table/issues/5461). Thanks to @TMRHarrison for the request with proposed fix, and Benjamin Schwendinger for the PR.

15. `fwrite` now returns the file path after writing data, [#5706](https://github.com/Rdatatable/data.table/issues/5706). This enhancement allows users to easily capture and utilize the file path for subsequent operations.Thanks to @Nj221102 for the PR.
Nj221102 marked this conversation as resolved.
Show resolved Hide resolved

## BUG FIXES

1. `unique()` returns a copy the case when `nrows(x) <= 1` instead of a mutable alias, [#5932](https://github.com/Rdatatable/data.table/pull/5932). This is consistent with existing `unique()` behavior when the input has no duplicates but more than one row. Thanks to @brookslogan for the report and @dshemetov for the fix.
Expand Down
6 changes: 3 additions & 3 deletions R/fwrite.R
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
if (file.exists(file)) {
suggested <- if (append) "" else gettextf("\nIf you intended to overwrite the file at %s with an empty one, please use file.remove first.", file)
warningf("Input has no columns; doing nothing.%s", suggested)
return(invisible())
return(invisible(file))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe here NULL should be returned, since the actual file is not affected at all by fwrite()? WDYT @eliocamp

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a though one. Honestly this behaviour is kind of unexpected to me. I would've expected fwrite to either write an empty file or to throw an error.

With append == TRUE, the file is left untouched, which would be the expected behaviour when writing an empty data.table, so returning the file would make sense, though.

} else {
warningf("Input has no columns; creating an empty file at '%s' and exiting.", file)
file.create(file)
return(invisible())
return(invisible(file))
}
}
yaml = if (!yaml) "" else {
Expand Down Expand Up @@ -118,7 +118,7 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
.Call(CfwriteR, x, file, sep, sep2, eol, na, dec, quote, qmethod=="escape", append,
row.names, col.names, logical01, scipen, dateTimeAs, buffMB, nThread,
showProgress, is_gzip, bom, yaml, verbose, encoding)
invisible()
invisible(file)
}

haszlib = function() .Call(Cdt_has_zlib)
28 changes: 14 additions & 14 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -9977,7 +9977,7 @@ test(1658.31, fwrite(ok_dt, qmethod=c("double", "double")), error="length(qmetho
test(1658.32, fwrite(ok_dt, col.names="foobar"), error="isTRUEorFALSE(col.names)")

# null data table (no columns)
test(1658.33, fwrite(data.table(NULL)), NULL, warning="Nothing to write")
test(1658.33, fwrite(data.table(NULL)), "", warning="Nothing to write")
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, wonder how useful this case is too -- when fwrite() to the terminal, maybe NULL should be returned as well? Not sure any inconsistency is worthwhile for this case though.


test(1658.34, fwrite(data.table(id=c("A","B","C"), v=c(1.1,0.0,9.9))), output="id,v\nA,1.1\nB,0\nC,9.9")

Expand Down Expand Up @@ -10009,9 +10009,9 @@ if (!haszlib()) {
} else {
test(1658.41, fwrite(data.table(a=c(1:3), b=c(1:3)), compress="gzip"), output='a,b\n1,1\n2,2\n3,3') # compress ignored on console
DT = data.table(a=rep(1:2,each=100), b=rep(1:4,each=25))
test(1658.421, fwrite(DT, file=f1<-tempfile(fileext=".gz"), verbose=TRUE), NULL,
test(1658.421, filepath <- fwrite(DT, file=f1<-tempfile(fileext=".gz"), verbose=TRUE), filepath,
Nj221102 marked this conversation as resolved.
Show resolved Hide resolved
output="args.nrow=200 args.ncol=2.*maxLineLen=5[12].*Writing 200 rows in 1 batches of 200 rows.*nth=1") # [12] for Windows where eolLen==2
test(1658.422, fwrite(DT, file=f2<-tempfile()), NULL)
test(1658.422, filepath <- fwrite(DT, file=f2<-tempfile()), filepath)
test(1658.423, file.info(f1)$size < file.info(f2)$size) # 74 < 804 (file.size() isn't available in R 3.1.0)
if (test_R.utils) test(1658.43, fread(f1), DT) # use fread to decompress gz (works cross-platform)
fwrite(DT, file=f3<-tempfile(), compress="gzip") # compress to filename not ending .gz
Expand Down Expand Up @@ -10046,7 +10046,7 @@ unlink(c(f1, f2))
# compression error -5 due to only 3 bytes (bom) in first block; #3599
if (haszlib()) {
DT = data.table(l=letters, n=1:26)
test(1658.53, fwrite(DT, file=f<-tempfile(fileext=".gz"), bom=TRUE, col.names=FALSE), NULL)
test(1658.53, filepath <- fwrite(DT, file=f<-tempfile(fileext=".gz"), bom=TRUE, col.names=FALSE), filepath)
if (test_R.utils) test(1658.54, fread(f), setnames(DT,c("V1","V2")))
unlink(f)
}
Expand All @@ -10059,7 +10059,7 @@ test(1658.56, fwrite(data.table(exp(1) - pi*1i)), output='2.718[0-9]*-3.141[0-9]
DT = data.table(a=1:3, b=list(1:4, c(3.14, 100e10), c(3i,4i,5i)))
test(1658.57, fwrite(DT), output='0+3i|0+4i|0+5i')
DT[ , b := c(1i, -1-1i, NA_complex_)]
test(1658.58, fwrite(DT), output='a,b\n1,0\\+1i\n2,-1-1i\n3,$')
test(1658.58, fwrite(DT), output='a,b\n1,0+1i\n2,-1-1i\n3,\n[1] ""')
Nj221102 marked this conversation as resolved.
Show resolved Hide resolved

# more coverage
test(1658.59, fwrite(data.table(a=list('a')), verbose=TRUE),
Expand Down Expand Up @@ -10809,7 +10809,7 @@ if (test_bit64) {
test(1731.1, class(DT[[1L]]), "integer64")
test(1731.2, fwrite(DT,na="__NA__"), output=ans)
f = tempfile()
test(1731.3, fwrite(DT, f, na="__NA__"), NULL)
test(1731.3, filepath <- fwrite(DT, f, na="__NA__"), filepath)
Nj221102 marked this conversation as resolved.
Show resolved Hide resolved
test(1731.4, readLines(f), ans)
unlink(f)
ans[1] = "V1" # the field is unquoted under `quote=FALSE`
Expand Down Expand Up @@ -10910,7 +10910,7 @@ test(1736.09, capture.output(fwrite(DT)), c("A,B", "foo,1|2|3", "\"ba|r\",1|2|3|
test(1736.10, capture.output(fwrite(DT,quote=TRUE)), c("\"A\",\"B\"", "\"foo\",1|2|3", "\"ba|r\",1|2|3|4", "\"baz\",\"fo|o\"|\"ba,r\"|\"baz\""))

# any list of same length vector input
test(1737.1, fwrite(list()), NULL, warning="fwrite was passed an empty list of no columns")
test(1737.1, fwrite(list()), "", warning="fwrite was passed an empty list of no columns")
test(1737.2, fwrite(list(1.2)), output="1.2")
test(1737.3, fwrite(list(1.2,B="foo")), output=",B\n1.2,foo")
test(1737.4, fwrite(list("A,Name"=1.2,B="fo,o")), output="\"A,Name\",B\n1.2,\"fo,o\"")
Expand Down Expand Up @@ -13089,11 +13089,11 @@ unlink(f)
#fwrite creates a file or does nothing, as appropriate, also #2898
DT = data.table(NULL)
f = tempfile()
test(1922.7, fwrite(DT, f), NULL, warning = 'no columns; creating an empty file')
test(1922.7, filepath <- fwrite(DT, f), filepath, warning = 'no columns; creating an empty file')
## above test created a file; now test behavior when file exists
test(1922.8, fwrite(DT, f), NULL, warning = 'no columns; doing nothing.*file.remove')
test(1922.8, filepath <- fwrite(DT, f), filepath, warning = 'no columns; doing nothing.*file.remove')
## slightly different behavior if append = TRUE
test(1922.9, fwrite(DT, f, append = TRUE), NULL, warning = 'doing nothing.$')
test(1922.9, filepath <- fwrite(DT, f, append = TRUE), filepath, warning = 'doing nothing.$')

# create index even if key present by setting attribute, #2883
DT = data.table(1:5, 1:5)
Expand Down Expand Up @@ -13141,8 +13141,8 @@ test(1937.1, DT[A %between% c(B,B+1)], error='RHS has length().*Perhaps you mean
test(1937.2, DT[A %between% B], error='length 2. The first')

# that fwrite'ing a list to a file works (it broke in dev 1.11.5 and was caught before release), PR#3017
test(1938.1, fwrite(list(1:3)), NULL, output="1\n2\n3") # never broke
test(1938.2, fwrite(list(1:3), file=f<-tempfile()), NULL) # just adding file= was what broke in dev just when x is list and not data.table|frame
test(1938.1, fwrite(list(1:3)), "", output="1\n2\n3") # never broke
test(1938.2, filepath <- fwrite(list(1:3), file=f<-tempfile()), filepath) # just adding file= was what broke in dev just when x is list and not data.table|frame
test(1939.3, readLines(f), as.character(1:3))
unlink(f)

Expand Down Expand Up @@ -13775,9 +13775,9 @@ if (.Platform$OS.type=="windows") {
test(1966.4, list.files(path = pth, pattern = "\\.csv$"), f)
unlink(c(fp, file.path(pth, "\u00c3\u00b6.csv")))
p = file.path(pth, "\u00fc"); dir.create(p); f = tempfile(tmpdir = p)
test(1966.5, fwrite(DT, enc2native(f)), NULL)
test(1966.5, filepath <- fwrite(DT, enc2native(f)), filepath)
Nj221102 marked this conversation as resolved.
Show resolved Hide resolved
unlink(f)
test(1966.6, fwrite(DT, enc2utf8(f)), NULL)
test(1966.6, filepath <- fwrite(DT, enc2utf8(f)), filepath)
unlink(p, recursive = TRUE)
}

Expand Down
Loading