Skip to content
New issue

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

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

Already on GitHub? # to your account

export several files to zip file (#203) #346

Merged
merged 11 commits into from
Sep 7, 2023

Conversation

schochastics
Copy link
Contributor

No description provided.

@schochastics
Copy link
Contributor Author

@chainsawriot please have a look if the logic I used makes sense to you

@schochastics schochastics changed the title export sevarl files to zip file (#203) export several files to zip file (#203) Sep 7, 2023
Copy link
Collaborator

@chainsawriot chainsawriot left a comment

Choose a reason for hiding this comment

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

Just two comments

R/export_list.R Outdated
@@ -76,14 +78,20 @@ function(
}
outfiles <- file
}

if (is.na(archive_format$compress) & archive_format$file != "") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Use && (gp)

  2. This won't write to new directory:

## if archivea is not created
export_list(mylist, file = paste0("file_", 1:3, ".xlsx"), archive = "archivea")

Maybe check and create it first?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, I suggest adding normalizePath to outfiles and archive. Windows cannot handle "~/" without normalizePath

outfiles <- base::normalizePath(outfiles)
archive <- base::normalizePath(archive) ## if it is not ""

ropensci/readODS#157

@chainsawriot
Copy link
Collaborator

@schochastics I checked and it works fine, except the case of writing to new directory.

@schochastics
Copy link
Contributor Author

@chainsawriot Thanks will address the comments

@schochastics I checked and it works fine, except the case of writing to new directory.

Yes forgot to ask you about that. export does not create a new dir if it doesn't exist

R> export(mtcars,"i_dont_exist/test.csv")
Error in data.table::fwrite(x, file = file, sep = sep, row.names = row.names,  : 
  No such file or directory: 'i_dont_exist/test.csv'. Unable to create new file for writing (it does not exist already). Do you have permission to write here, is there space on the disk and does the path exist?

I can make export_list create a new dir if it doesnt exist, but then we probably should also do it for export?

@chainsawriot
Copy link
Collaborator

@schochastics Thanks for finding this out. I will open a new issue and fix that.

@schochastics
Copy link
Contributor Author

@chainsawriot actually since export_list calls export for the default case, I will only implement the "new dir" feature for the archive

@chainsawriot
Copy link
Collaborator

@schochastics OK, just leave it like that and I will fix #347

I think the expected output for test_export_list.R:25:5 should be the normalizedPath(archive)

@chainsawriot
Copy link
Collaborator

@schochastics Great! Thank you!

@chainsawriot chainsawriot marked this pull request as ready for review September 7, 2023 12:54
@chainsawriot chainsawriot merged commit e0f133a into gesistsa:main Sep 7, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants