-
Notifications
You must be signed in to change notification settings - Fork 59
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
IJulia docstrings + tests #542
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #542 +/- ##
==========================================
+ Coverage 80.65% 81.14% +0.49%
==========================================
Files 26 26
Lines 1701 1671 -30
==========================================
- Hits 1372 1356 -16
+ Misses 329 315 -14 ☔ View full report in Codecov by Sentry. |
RCall.ijulia_init() | ||
R"plot(1:10, 1:10)" | ||
# throws a method error when running headless ! | ||
@test_throws(MethodError, RCall.ijulia_displayplots()) |
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.
@ararslan if you have a better idea, I'd be keen. I considered pirating (in the tests) display(::MIME, Vector{UInt8})
but that seemed even worse
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 don't think I fully understand the context. What's the desired/typical behavior? If you want to test that it throws a method error then this seems fine...
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.
When called in a Jupyter notebook, this will result in the R plot (written to a temporary file) being read in and then display
ed using the appropriate MIME type. But when called in a plain text, headless environment, you get a method error. I'm not too worried about it since we're testing the actual behavior separately (via the nbconvert
stuff above), but not in a way that codecov detects it (due to the nesting of processes). So the run here is just re-running the same code that's in the notebook so that codecov gets it.
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 know basically nothing about IJulia and have used it no more than twice ever, I believe, so if it's a meaningful review you're after, I'm not your guy. 😛 However, I have this rubber stamp next to me and the ink is still wet...
RCall.ijulia_init() | ||
R"plot(1:10, 1:10)" | ||
# throws a method error when running headless ! | ||
@test_throws(MethodError, RCall.ijulia_displayplots()) |
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 don't think I fully understand the context. What's the desired/typical behavior? If you want to test that it throws a method error then this seems fine...
Co-authored-by: Alex Arslan <[email protected]>
closes #141