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

why have separate path and file arguments to the _latex function? #29

Closed
drmowinckels opened this issue Sep 12, 2022 · 3 comments · Fixed by #30
Closed

why have separate path and file arguments to the _latex function? #29

drmowinckels opened this issue Sep 12, 2022 · 3 comments · Fixed by #30

Comments

@drmowinckels
Copy link

This is part of the openjournals/joss-reviews#4740 .

I was wondering why there is a need to specify both destination folder and filename for saving a file? This is quite uncommon when saving files in R, where we would usually provide the entire path as a single argument.

@cosimameyer
Copy link
Owner

Thanks for the suggestion, @drmowinckels :)

We created the function to work with one base path for multiple outputs. Coming from a social sciences background, you might want to generate overview tables for multiple subsets of your data set. Creating a file path upfront in the script and adding the file_path object in the overview_latex function (thanks so much for suggesting the new name, I love it! 🥰 ) allows us to avoid path copying.

file_path <- 'some/file_path/'

overview_latex(
   obj = overview_table1,
   path = file_path,
   file = 'plot1.tex'
)

overview_latex(
   obj = overview_table2,
   path = file_path,
   file = 'plot2.tex'
)    

An alternative that I can think of, on the top of my head, would be to rewrite the function as follows:

file_path <- 'some/file_path/'

overview_latex(
   obj = overview_table1,
   path = paste0(file_path, 'plot1.tex')
)

overview_latex(
   obj = overview_table2,
   path = paste0(file_path, 'plot2.tex')
)    

I agree that it makes it shorter, but I'm not sure if it makes it cleaner. But I'm curious what you think 😊

@drmowinckels
Copy link
Author

I think there are pros and cons for each, and having created a package that does indeed take a folder as a destination I understand what you are thinking. I too am from social science (psychology), so I totally get where you are coming from.

At the same time, I think a folder makes sense if the function it self creates multiple files. But if a single call to the function creates a single file, then giving the full (or relative) path makes more sense. And then its up to the user to do as you do.

Think of how other writing functions work, where they take a single path for a single file. write.table, readr::write_tsv, etc. We are used to having to provide a single path for a single function call already. As a user, if I were just writing a single file, I'd find it strange to provide both a path and destination folder, I'd for sure do it incorrectly at the first try because of this. From an R point of view, I think a single path makes sense.

That being said. I am not going to force a change on this at all, I merely wanted a discussion around it, as I found it odd. It is not a big deal at all, and if you are happy with it, then go for it. If you, after reading this comment, still want to keep it as is, then you can close the comment, and I'll finish my review.

@cosimameyer
Copy link
Owner

cosimameyer commented Sep 12, 2022

Thanks for your response and for sharing your thoughts. We find the reasoning very convincing and highly appreciate getting an outside perspective on our package's functionality :)
We changed the argument from file and path to a single argument file_path in PR #30 and #32 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants