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

Use fs for all file path operations #38

Closed
mpadge opened this issue Aug 29, 2024 · 4 comments
Closed

Use fs for all file path operations #38

mpadge opened this issue Aug 29, 2024 · 4 comments

Comments

@mpadge
Copy link
Collaborator

mpadge commented Aug 29, 2024

I recently got an email about gtfsrouter luckily with reprex pasted in to show that file paths on windows were unusable, just because of combining temp.dir() with file.path(). Using fs solves those issues be standardising paths on windows, which I suggest would also be good to implement here. Happy to do this if nobody else is?

@r-transit r-transit deleted a comment from jungminjun824 Aug 29, 2024
@polettif
Copy link
Collaborator

Sounds good, thanks 👍 I probably should look into fs, haven't used it so far.

@rafapereirabr
Copy link
Collaborator

rafapereirabr commented Aug 29, 2024

Hi @mpadge , thanks for the heads up ! Could you please share the reprex they sent you?

ps. I usually use tempdir() and tools::R_user_dir() in others packages but I haven't come across any issues so far.

@mpadge
Copy link
Collaborator Author

mpadge commented Aug 30, 2024

The reprex was this:

> f <- berlin_gtfs_to_zip ()
> gtfs <- extract_gtfs (f, quiet = TRUE)

#> Errore in check_extract_pars(filename, stn_suffixes) :
#>  filename C:\Users\<User>\AppData\Local\Temp\Rtmp4GhleE/vbb.zip does not exist

The filepath for f constructed in berlin_gtfs_to_zip() just used file.path(tempdir(), "vbb.zip"). And on windows that led to tempdir() using \, but file.path() appending filename with /. This can always happen with windows systems, which was one of the motivations for the fs package. fs always normalises paths to use consistent deilimiters. The package has no additional deps at all, is very lightweight, and has 1-1 replacements for all standard functions:

fs::path(fs::path_temp(), "vbb.zip")

@rafapereirabr
Copy link
Collaborator

Thanks for sharing, @mpadge !

@polettif polettif mentioned this issue Oct 7, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 8, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 9, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 11, 2024
mpadge added a commit to mpadge/gtfsio that referenced this issue Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants