-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add workaround for displaying ellipsis on windows #43
Conversation
Lets see if my changes to the unittests let this pass... on windows, they don't :-( |
ellip.h <- '\u22EF' | ||
|
||
# copy of the function repr_matrix_df.r | ||
.char_fallback <- function(char, default) { |
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.
no duplicate code. please put it into utils.r.
and btw, why can’t the tests just use the ellip.{hvd} from repr_matrix_df?
aren’t tests executed in the package namespace and have private access?
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've no clue, but the tests are anyway failing for me on windows (and I've no clue why). I tried putting the function in only once as a top level function in the test file, but that didn't work either and I don't want to export the function just to make the tests happy...
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.
what do you mean with “work” here?
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.
didn't work = the test failed even more... (actually I was walking in the dark with this: I've no clue what and why test failed on windows) If you tell me that adding the function on top of the file will still let it pass on travis, that's fine with me.
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.
ok, the fact that you don’t know is weird: shouldn’t the test runner tell you?
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.
It tells me that the test is broken, but it didn't show me what specific was broken (basically everything but the matrix comparison in the last testcase were :-()
I haven't worked with test_that yet, so no clue how to get better output or how to debug this stuff :-/
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.
something’s fucky. you should see sth like here.
how do you run the tests?
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.
R CMD check ...
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.
as you can see in the link, that’s what travis uses to produce that output, as well. hmm.
anyway: the testthat package has a function to run the tests, you can try that one.
These are the failures (now via Rstudio): ==> devtools::test()
Loading repr
Loading required package: testthat
Loading required namespace: Cairo
Testing repr
Array and vector truncation: ..............
Attaching package: 'dplyr'
The following objects are masked from 'package:data.table':
between, last
The following objects are masked from 'package:stats':
filter, lag
The following objects are masked from 'package:base':
intersect, setdiff, setequal, union
................................WWWW.123WWWW.456WWWW.789
LaTeX and HTML escaping: ..............abcd....efFF
Warnings -------------------------------------------------------------------
1. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#318) - duplicated levels in factors are deprecated
2. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#319) - duplicated levels in factors are deprecated
3. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#320) - duplicated levels in factors are deprecated
4. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#325) - duplicated levels in factors are deprecated
5. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#340) - duplicated levels in factors are deprecated
6. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#341) - duplicated levels in factors are deprecated
7. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#342) - duplicated levels in factors are deprecated
8. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#348) - duplicated levels in factors are deprecated
9. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#363) - duplicated levels in factors are deprecated
10. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#364) - duplicated levels in factors are deprecated
11. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#365) - duplicated levels in factors are deprecated
12. ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#372) - duplicated levels in factors are deprecated
Failed ---------------------------------------------------------------------
1. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#333)
`limited_df` not equal to `expected_df`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
2. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#334)
`limited_dt` not equal to `expected_dt`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
3. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#335)
`limited_tbl` not equal to `expected_tbl`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
4. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#356)
`limited_df` not equal to `expected_df`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
5. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#357)
`limited_dt` not equal to `expected_dt`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
6. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#358)
`limited_tbl` not equal to `expected_tbl`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
7. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#380)
`limited_df` not equal to `expected_df`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
8. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#381)
`limited_dt` not equal to `expected_dt`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
9. Failure: ellip.limit.arr limits arrays that are long and wide (@test_array_manipulation.r#382)
`limited_tbl` not equal to `expected_tbl`.
Component "...": Attributes: < Component "levels": Lengths (2, 3) differ (string compare on first 2) >
Component "...": Attributes: < Component "levels": 1 string mismatch >
10. Failure: LaTeX escaping in vectors works (@test_escaping.r#24) -------------
repr_latex("[") not equal to "'{[}'".
1/1 mismatches
x[1]: "\"{[}\""
y[1]: "'{[}'"
11. Failure: LaTeX escaping in vectors works (@test_escaping.r#25) -------------
repr_latex(c("[", "|")) not equal to "\\begin{enumerate*}\n\\item '{[}'\n\\item '\\textbar{}'\n\\end{enumerate*}\n".
1/1 mismatches
x[1]: "\\begin{enumerate*}\n\\item \"{[}\"\n\\item \"\\textbar{}\"\n\\end{enumer
x[1]: ate*}\n"
y[1]: "\\begin{enumerate*}\n\\item '{[}'\n\\item '\\textbar{}'\n\\end{enumerate*
y[1]: }\n"
12. Failure: HTML escaping in vectors works (@test_escaping.r#34) --------------
repr_html("<") not equal to "'<'".
1/1 mismatches
x[1]: "\"<\""
y[1]: "'<'"
13. Failure: HTML escaping in vectors works (@test_escaping.r#35) --------------
repr_html(c("<", "&")) not equal to "<ol class=list-inline>\n\t<li>'<'</li>\n\t<li>'&'</li>\n</ol>\n".
1/1 mismatches
x[1]: "<ol class=list-inline>\n\t<li>\"<\"</li>\n\t<li>\"&\"</li>\n</ol>\
x[1]: n"
y[1]: "<ol class=list-inline>\n\t<li>'<'</li>\n\t<li>'&'</li>\n</ol>\n"
14. Failure: LaTeX escaping in lists works (@test_escaping.r#80) ---------------
repr_latex(list(lbr = "[")) not equal to "\\textbf{\\$lbr} = '{[}'".
1/1 mismatches
x[1]: "\\textbf{\\$lbr} = \"{[}\""
y[1]: "\\textbf{\\$lbr} = '{[}'"
15. Error: LaTeX escaping in lists works (@test_escaping.r#81) -----------------
too few arguments
1: expect_equal(repr_latex(list(`&` = "%")), "\\textbf{\\$`\\&`} = '\\%'") at C:\data\external\R\repr/tests/testthat/test_escaping.r:81
2: compare(object, expected, ...)
3: compare.character(object, expected, ...)
4: difference(format(mismatches, max_diffs = max_diffs, max_lines = max_lines, width = width))
5: comparison(FALSE, sprintf(...))
6: stopifnot(is.character(message), length(message) == 1)
7: sprintf(...)
DONE =======================================================================
Not sure if I executed the test in the right way... |
SO, IMO the unittest failures on win and the ggplot error logging are the only blocker for a release. |
And it is green on windows... Lets see what travis tells us... |
@@ -5,6 +5,17 @@ library(data.table) | |||
library(dplyr) | |||
options('stringsAsFactors' = FALSE) | |||
|
|||
# copy of the function repr_matrix_df.r | |||
.char_fallback <- function(char, default) { |
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.
can you really not use the one from the package? maybe do ellip.h <- IRkernel:::ellip.h` (and so on)?
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.
done
Doing this all in RStudio is make this much easier...
On windows, ellipses are usually outside the chars which can be displayed in our way to display plain text /with `capture.output(print(...))` in the current locale. The leads to strangely escaped chars and we would have such a escaped chars each time we display a dataframe. This problem is probably not going away soon: IRkernel#28 (comment) Work around this by simple using three dots.
The tests were failing because for unknown reasons, the repr_xxx was returning inline quotes signs as " instead of the expected '. The easiest fix is to substitute that in a special expect_equivalent_string function.
travis is green, all redundant functions and fields were remove. If @flying-sheep doesn't find any style problems, it's IMO ready to be merged. |
On windows, ellipses are usually outside the chars which can be displayed in our
way to display plain text /with
capture.output(print(...))
in the currentlocale. The leads to strangely escaped chars and we would have such a escaped
chars each time we display a dataframe.
This problem is probably not going away soon:
#28 (comment)
Work around this by simple using three dots.