Changed conversion, added tests #145
Conversation
Codecov Report
@@ Coverage Diff @@
## master #145 +/- ##
=========================================
+ Coverage 55.99% 56.4% +0.41%
=========================================
Files 44 44
Lines 2611 2599 -12
=========================================
+ Hits 1462 1466 +4
+ Misses 1149 1133 -16
|
sebasguts
force-pushed
the
new_conversion
branch
from
22685d5 to
e288ca5
Compare
sebasguts
changed the title
There was a problem hiding this comment.
Clearly this is an improvement.
But I worry about the fact that we are implementing separate conversions in the GAP / C side and the Julia side; not only does this duplicate a lot of work, it also means that the two are not behaving the same.
We should think about a way that allows us to implement all these conversions once (either in C or Julia; can be mixed, depending on the situation, too); and then share these implementations between LibGAP.jl and JuliaInterface.
That's not for this PR, though, but rather something I'll put into an issue.
| from_gap_string(obj :: MPtr) = CSTR_STRING(obj) | ||
| from_gap_symbol(obj :: MPtr) = Symbol(from_gap_string(obj)) | ||
| from_gap(obj , ::Any) = obj | ||
| from_gap(obj :: MPtr, ::Type{AbstractString}) = CSTR_STRING(obj) |
There was a problem hiding this comment.
As per the comment above, this (still) does not perform any input validation and will crash or produce garbage if called on a non-string GAP object. Something to fix in a future PR, I guess.
|
Agreed. I will provide a PR which uses the C functions instead. |
No description provided.