-
Notifications
You must be signed in to change notification settings - Fork 985
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 tests for complex #5796
Fix tests for complex #5796
Conversation
@@ -10964,7 +10964,7 @@ test(1743.217, sapply(fread("a,b,c,d,e,f\na,b,c,d,e,f", colClasses = list(factor | |||
test(1743.218, sapply(fread("a,b,c,d,e,f\na,b,c,d,e,f", colClasses = list(factor = c(1, 2, 4), factor = 3), select = c(5, 4, 2, 3)), class), y = c(e = "character", d = "factor", b = "factor", c = "factor")) | |||
|
|||
test(1743.22, fread("a,b,c\n1999/01/01,2,f", colClasses=list(Date=1L), drop="a"), data.table(b=2L, c="f")) | |||
test(1743.231, fread("a,b,c\n2,1,4i", colClasses=list(complex="c", integer=2L), drop="a"), data.table(b=1L, c="4i"), | |||
test(1743.231, fread("a,b,c\n2,1,4j", colClasses=list(complex="c", integer=2L), drop="a"), data.table(b=1L, c="4j"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a substantial improvement in r-devel -- previously it seems as.complex("4i")
would fail to coerce which seems wrong. Better to try and coerce to complex something that's not actually complex, to ensure the warning continues.
cc original author @HughParsonage in case I'm missing something behind the intent of this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Matt added that specific test, though I added the tests around it, which looks to have been a back and forth that observed but did not mention the behaviour of as.complex("4i"):
c6e2b72#diff-e3243f3780ce7d303c3317f73945310bfc37e45d193568246246aca20e3270ae
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5796 +/- ##
=======================================
Coverage 97.46% 97.46%
=======================================
Files 80 80
Lines 14814 14814
=======================================
Hits 14439 14439
Misses 375 375 ☔ View full report in Codecov by Sentry. |
Part of #5748 (fixed for master, not hotfix)