Closed NilsEnevoldsen closed 2 years ago
Wonderful, thanks so much! Should I merge now?
Samuel (who did the new regex sets) sent me a bunch of tiny suggestions by email. I'll do those when I come back and then release to CRAN.
Don't merge; there's a failing test in AppVeyor that is one I just added.
I don't understand why it's not failing for me or for the GitHub Actions.
Also I have a local warning that these don't have:
==> Sourcing R files in 'tests' directory
Loading required package: countrycode
[ FAIL 0 | WARN 1 | SKIP 0 | PASS 3741 ]
══ Warnings ════════════════════════════════════════════════════════════════════
── Warning (test-countryname.R:13:5): output types ─────────────────────────────
The origin and destination codes are not of the same
class. Filling-in bad matches with NA instead.
Backtrace:
1. testthat::expect_identical(...)
at test-countryname.R:13:4
4. countrycode::countryname("ジンバブエ", "cown")
5. countrycode::countrycode(...)
at countrycode/R/countryname.R:45:6
7. countrycode:::countrycode_convert(...)
at countrycode/R/countrycode.R:202:8
[ FAIL 0 | WARN 1 | SKIP 0 | PASS 3741 ]
Tests complete
What do you see if you run countrycode::countryname("ジンバブエ", "cown")
?
I'm on mobile, sorry.
FWIW, I've always had (often unexplainable) trouble with appveyor and I don't use it in newer projects.
Sorry it took so long. I get the same error:
> countrycode::countryname("ジンバブエ", "cown")
[1] 552
Warning message:
In countrycode_convert(sourcevar = sourcevar, origin = origin, destination =
dest, :
The origin and destination codes are not of the same
class. Filling-in bad matches with NA instead.
Edit: this is with the main branch of your fork, on a Ubuntu WSL2 virtual machine inside windows.
Edit: this is with the main branch of your fork, on a Ubuntu WSL2 virtual machine inside windows.
What do you get if you use the version on CRAN? I get a little confused by package management, but I think I'm getting this error on the deployed package (on macOS, anyway).
Yep, I seem to get the same warning with CRAN version.
Okay to merge then.
I opened #309 about the "real" warning I saw, and I'm just going to ignore the Appveyor issue, hoping that it's a test-environment-only Unicode issue.
Thanks a ton!
And sorry for the delayed responses. I was on vacation without regular computer access.
UN and WB scrapes automatically pulled the new name for Türkiye. I modified the Türkiye regex for English and French manually.
Fixes #307.