-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use julia_gap and gap_julia in more places #127
Conversation
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
==========================================
+ Coverage 54.33% 54.34% +0.01%
==========================================
Files 44 44
Lines 2632 2622 -10
==========================================
- Hits 1430 1425 -5
+ Misses 1202 1197 -5
|
6458f23
to
f330002
Compare
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 looks good.
Note that this changes some things, and might cause some problems with JuliaExperimental
.
How do we want to proceed: Change a lot in JuliaInterface
and then make JuliaExperimental
catch up later?
@ThomasBreuer what do you think?
f330002
to
b194557
Compare
My personal stance on this is this: This PR updates the existing tests for JuliaExperimental, so code covered by that is fine. All the other code in |
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.
Sorry, but some tests are failing. Should I correct them?
@sebasguts Yes, merging proposed changes in JuliaInterface is fine, independent of what they do with JuliaExperimental. |
... for consistency in the auto-conversion behaviour of JuliaInterface versus LibGAP.jl, and for ease of use. There is more work to be done here, though, and we need to properly document the rules for when and where which conversion is applied, and why.
b194557
to
55cce99
Compare
Those test failures were real bugs, yay to the new tests that helped uncover them before merging! @ThomasBreuer of course I am assuming that any adjustment to JuliaExperimental tests would involve some reasonable thinking, and questioning of what is there. Still, for tests to be most effective, it is important that they all pass (resp. that tests which are expected to fail are marked as such). |
... for consistency in the auto-conversion behaviour of JuliaInterface versus
LibGAP.jl, and for ease of use.
There is more work to be done here, though, and we need to properly
document the rules for when and where which conversion is applied,
and why.