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

Fix for failing test for UTF-8 encoded filenames on Windows with R 3.5 #340

Closed
wants to merge 2 commits into from

Conversation

gergness
Copy link

@gergness gergness commented Jun 7, 2018

Adapts the same logic that fixed a similar problem in readxl because of the change in behavior of normalizePaths() in R 3.5.

@gergness
Copy link
Author

gergness commented Jun 8, 2018

I don't think that travis error is related to my changes -- it looks like the machine was just too busy. Let me know if you want me to investigate further.

@wesm
Copy link
Owner

wesm commented Jun 11, 2018

@hadley do we want to pull in accumulated R 3.5 fixes and make a 0.3.2 release?

@hadley
Copy link
Collaborator

hadley commented Jun 13, 2018

Yeah, I think that's a good idea.

R/R/class.R Outdated
@@ -7,7 +7,7 @@
#' @return An object of class \code{feather}
#' @export
feather <- function(path) {
path <- normalizePath(path, mustWork = TRUE)
path <- enc2native(path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not an adequate replacement to normalisePath() as it won't replace ~

Copy link
Author

Choose a reason for hiding this comment

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

Oh okay - new commit fixes the error but still normalizes. (FWIW the old code followed readxl's lead - tidyverse/readxl#477 takes out normalizePath())

@codecov-io
Copy link

Codecov Report

Merging #340 into master will decrease coverage by 0.93%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #340      +/-   ##
=========================================
- Coverage   78.33%   77.4%   -0.94%     
=========================================
  Files           7       7              
  Lines         554     540      -14     
=========================================
- Hits          434     418      -16     
- Misses        120     122       +2
Impacted Files Coverage Δ
R/class.R 76.47% <0%> (-7.66%) ⬇️
R/feather.R 57.14% <0%> (-5.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be5141b...91b9315. Read the comment docs.

@gergness
Copy link
Author

This PR was superseded, thanks!

@gergness gergness closed this Jan 16, 2019
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

Successfully merging this pull request may close these issues.

4 participants