Use julia_gap and gap_julia in more places

... 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.
oscar-system · Nov 5, 2018 · 55cce99 · 55cce99
1 parent fd01ea5
commit 55cce99
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 50 deletions.
diff --git a/JuliaExperimental/tst/realcyc.tst b/JuliaExperimental/tst/realcyc.tst
@@ -37,7 +37,7 @@ false
 
 ##
 gap> Julia.GAPRealCycModule.test_this_module();
-<Julia: true>
+true
 
 ##
 gap> STOP_TEST( "realcyc.tst", 1 );

diff --git a/JuliaInterface/gap/JuliaInterface.gd b/JuliaInterface/gap/JuliaInterface.gd
@@ -44,7 +44,7 @@ DeclareOperation( "ConvertedToJulia", [ IsObject ] );
 #! @Description
 #!  Retuns an object that is a sensible conversion from the Julia object <A>julia_object</A>.
 #!  If no conversion exists, <C>fail</C> is returned.
-DeclareOperation( "ConvertedFromJulia", [ IsJuliaObject ] );
+DeclareOperation( "ConvertedFromJulia", [ IsObject ] );
 
 #! @Arguments julia_object
 #! @Returns an object
@@ -139,5 +139,3 @@ DeclareGlobalFunction( "JuliaTypeInfo" );
 #!  If an error occured then <C>ok</C> has the value <K>false</K>,
 #!  and <C>value</C> is the error message as a &GAP; string.
 DeclareGlobalFunction( "CallJuliaFunctionWithCatch" );
-
-
diff --git a/JuliaInterface/gap/JuliaInterface.gi b/JuliaInterface/gap/JuliaInterface.gi
@@ -28,6 +28,10 @@ InstallMethod( ConvertedFromJulia,
                [ IsJuliaObject ],
     _ConvertedFromJulia );
 
+InstallMethod( ConvertedFromJulia,
+               [ IsObject ],
+    IdFunc );
+
 InstallMethod( ConvertedToJulia,
                 [ IsObject ],
   function( obj )
@@ -91,9 +95,9 @@ InstallMethod( \.,
         Error( rnam, " is not bound in Julia" );
     fi;
 
-    if ConvertedFromJulia( _JULIA_ISA( global_variable, _JULIA_FUNCTION_TYPE ) ) then
+    if _JULIA_ISA( global_variable, _JULIA_FUNCTION_TYPE ) then
         global_variable := _JuliaFunction( global_variable, true );
-    elif ConvertedFromJulia( _JULIA_ISA( global_variable, _JULIA_MODULE_TYPE ) ) then
+    elif _JULIA_ISA( global_variable, _JULIA_MODULE_TYPE ) then
         global_variable := _WrapJuliaModule( rnam, global_variable );
     fi;
 
@@ -195,7 +199,7 @@ InstallGlobalFunction( ImportJuliaModuleIntoGAP,
     is_module_present := JuliaEvalString( Concatenation( "isdefined( Main, :", name, ")" ) );
 
     if no_import = false then
-        if not ConvertedFromJulia( is_module_present ) then
+        if not is_module_present then
             ## Local modules cannot be imported
             callstring:= Concatenation( "import ", name );
             JuliaEvalString( callstring );
@@ -218,7 +222,7 @@ InstallGlobalFunction( JuliaImportPackage, function( pkgname )
     fi;
     callstring := Concatenation( "try import ", pkgname,
                      "; return true; catch e; return e; end" );
-    if ConvertedFromJulia( JuliaEvalString( callstring ) ) = true then
+    if JuliaEvalString( callstring ) = true then
         return true;
     else
       Info( InfoWarning, 1,
@@ -238,7 +242,7 @@ InstallGlobalFunction( CallJuliaFunctionWithCatch,
     local res;
 
     res:= Julia.GAPUtils.call_with_catch( juliafunc, arguments );
-    if ConvertedFromJulia( res[1] ) = true then
+    if res[1] then
       return rec( ok:= true, value:= res[2] );
     else
       return rec( ok:= false, value:= ConvertedFromJulia( res[2] ) );

diff --git a/JuliaInterface/src/JuliaInterface.c b/JuliaInterface/src/JuliaInterface.c
@@ -5,6 +5,7 @@
 #include "JuliaInterface.h"
 
 #include "calls.h"
+#include "convert.h"
 
 #include <src/compiled.h>    // GAP headers
 
@@ -215,7 +216,7 @@ static Obj FuncJuliaEvalString(Obj self, Obj string)
     // JL_TRY/JL_CATCH.
     jl_value_t * result = jl_eval_string(copy);
     JULIAINTERFACE_EXCEPTION_HANDLER
-    return NewJuliaObj(result);
+    return gap_julia(result);
 }
 
 // Converts the julia value pointer <julia_obj> into a GAP object
@@ -288,6 +289,8 @@ Obj _ConvertedFromJulia_internal(jl_value_t * julia_obj)
                 continue;
             }
             jl_value_t * current_jl_element = jl_arrayref(array_ptr, i);
+            // TODO: replace NewJuliaObj here by gap_julia, or
+            // by a recursive call to _ConvertedFromJulia_internal?
             current_element = NewJuliaObj(current_jl_element);
             SET_ELM_PLIST(return_list, i + 1, current_element);
             CHANGED_BAG(return_list);
@@ -455,8 +458,10 @@ static Obj Func_ConvertedFromJulia_record_dict(Obj self, Obj dict)
         real_index++;
         current_value = jl_arrayref(dict_values, i);
         current_index = jl_arrayref(dict_indices, i);
+        // FIXME: should we use gap_julia here instead of NewJuliaObj?
         SET_ELM_PLIST(values_gap, real_index, NewJuliaObj(current_value));
         CHANGED_BAG(values_gap);
+        // FIXME: should we use gap_julia here instead of NewJuliaObj?
         SET_ELM_PLIST(indices_gap, real_index, NewJuliaObj(current_index));
         CHANGED_BAG(indices_gap);
     }
@@ -529,18 +534,14 @@ static Obj FuncJuliaModule(Obj self, Obj name)
     return NewJuliaObj((jl_value_t *)julia_module);
 }
 
-// Sets the value of the julia identifier <name> to the julia value the julia
-// object GAP object <julia_val> points to.
-// This function is for debugging purposes
-static Obj FuncJuliaSetVal(Obj self, Obj name, Obj julia_val)
+// Sets the value of the julia identifier <name> to the <val>.
+// This function is for debugging purposes.
+static Obj FuncJuliaSetVal(Obj self, Obj name, Obj val)
 {
     if (!IsStringConv(name)) {
         ErrorMayQuit("JuliaSetVal: <name> must be a string", 0, 0);
     }
-    if (!IS_JULIA_OBJ(julia_val)) {
-        ErrorMayQuit("JuliaSetVal: <julia_val> must be a Julia object", 0, 0);
-    }
-    jl_value_t * julia_obj = GET_JULIA_OBJ(julia_val);
+    jl_value_t * julia_obj = julia_gap(val);
     jl_sym_t *   julia_symbol = jl_symbol(CSTR_STRING(name));
     jl_set_global(jl_main_module, julia_symbol, julia_obj);
     return 0;
@@ -560,7 +561,7 @@ static Obj Func_JuliaGetGlobalVariable(Obj self, Obj name)
         return Fail;
     }
     jl_value_t * value = jl_get_global(jl_main_module, symbol);
-    return NewJuliaObj(value);
+    return gap_julia(value);
 }
 
 // Returns the julia object GAP object that holds a pointer to the value
@@ -591,7 +592,7 @@ static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module)
         return Fail;
     }
     jl_value_t * value = jl_get_global(m, symbol);
-    return NewJuliaObj(value);
+    return gap_julia(value);
 }
 
 // Returns the julia object GAP object that holds a pointer to the value
@@ -617,7 +618,7 @@ static Obj FuncJuliaGetFieldOfObject(Obj self, Obj super_obj, Obj field_name)
     jl_value_t * field_value =
         jl_get_field(extracted_superobj, CSTR_STRING(field_name));
     JULIAINTERFACE_EXCEPTION_HANDLER
-    return NewJuliaObj(field_value);
+    return gap_julia(field_value);
 }
 
 

diff --git a/JuliaInterface/src/calls.c b/JuliaInterface/src/calls.c
@@ -101,10 +101,7 @@ static ALWAYS_INLINE Obj DoCallJuliaFunc(Obj       func,
     }
     else {
         for (int i = 0; i < narg; i++) {
-            if (IS_INTOBJ(a[i]))
-                a[i] = (Obj)jl_box_int64(INT_INTOBJ(a[i]));
-            else if (IS_FFE(a[i]))
-                ErrorQuit("TODO: implement conversion for T_FFE", 0, 0);
+            a[i] = (Obj)julia_gap(a[i]);
         }
     }
     jl_function_t * f = GET_JULIA_FUNC(func);
@@ -129,9 +126,7 @@ static ALWAYS_INLINE Obj DoCallJuliaFunc(Obj       func,
     // and its variants are part of the jlapi, so don't have to be wrapped in
     // JL_TRY/JL_CATCH.
     JULIAINTERFACE_EXCEPTION_HANDLER
-    if (IsGapObj(result))
-        return (Obj)result;
-    return NewJuliaObj(result);
+    return gap_julia(result);
 }
 
 static Obj DoCallJuliaFunc0ArgConv(Obj func)

diff --git a/JuliaInterface/tst/calls.tst b/JuliaInterface/tst/calls.tst
@@ -176,31 +176,31 @@ gap> fwN();
 
 #
 gap> fwN(true);
-<Julia: (GAP: true,)>
+<Julia: (true,)>
 
 #
 gap> fwN(true,2);
-<Julia: (GAP: true, 2)>
+<Julia: (true, 2)>
 
 #
 gap> fwN(true,2,3);
-<Julia: (GAP: true, 2, 3)>
+<Julia: (true, 2, 3)>
 
 #
 gap> fwN(true,2,3,4);
-<Julia: (GAP: true, 2, 3, 4)>
+<Julia: (true, 2, 3, 4)>
 
 #
 gap> fwN(true,2,3,4,5);
-<Julia: (GAP: true, 2, 3, 4, 5)>
+<Julia: (true, 2, 3, 4, 5)>
 
 #
 gap> fwN(true,2,3,4,5,6);
-<Julia: (GAP: true, 2, 3, 4, 5, 6)>
+<Julia: (true, 2, 3, 4, 5, 6)>
 
 #
 gap> fwN(true,2,3,4,5,6,7);
-<Julia: (GAP: true, 2, 3, 4, 5, 6, 7)>
+<Julia: (true, 2, 3, 4, 5, 6, 7)>
 
 # non-variadic functions
 gap> f0wN := _JuliaFunction("f0", false);;
@@ -218,31 +218,31 @@ gap> f0wN();
 
 #
 gap> f1wN(true);
-<Julia: (GAP: true,)>
+<Julia: (true,)>
 
 #
 gap> f2wN(true,2);
-<Julia: (GAP: true, 2)>
+<Julia: (true, 2)>
 
 #
 gap> f3wN(true,2,3);
-<Julia: (GAP: true, 2, 3)>
+<Julia: (true, 2, 3)>
 
 #
 gap> f4wN(true,2,3,4);
-<Julia: (GAP: true, 2, 3, 4)>
+<Julia: (true, 2, 3, 4)>
 
 #
 gap> f5wN(true,2,3,4,5);
-<Julia: (GAP: true, 2, 3, 4, 5)>
+<Julia: (true, 2, 3, 4, 5)>
 
 #
 gap> f6wN(true,2,3,4,5,6);
-<Julia: (GAP: true, 2, 3, 4, 5, 6)>
+<Julia: (true, 2, 3, 4, 5, 6)>
 
 #
 gap> f7wN(true,2,3,4,5,6,7);
-<Julia: (GAP: true, 2, 3, 4, 5, 6, 7)>
+<Julia: (true, 2, 3, 4, 5, 6, 7)>
 
 #
 # calls via wrapped C function pointers
@@ -280,17 +280,17 @@ Error, Only 0-6 arguments are supported
 gap> g0();
 <Julia: Ptr{Nothing} @0x0000000000000000>
 gap> g1(true);
-<Julia: true>
+true
 gap> g2(true,2);
-<Julia: 2>
+2
 gap> g3(true,2,3);
-<Julia: 3>
+3
 gap> g4(true,2,3,4);
-<Julia: 4>
+4
 gap> g5(true,2,3,4,5);
-<Julia: 5>
+5
 gap> g6(true,2,3,4,5,6);
-<Julia: 6>
+6
 
 #
 gap> g0C();
@@ -344,7 +344,7 @@ Error, TypeError: in h6, in cfunction, expected Ptr{Nothing}, got Nothing
 #
 gap> _NewJuliaCFunc(fail, fail);
 Error, NewJuliaCFunc: <ptr> must be a Julia object
-gap> _NewJuliaCFunc(JuliaEvalString("1"), fail);
+gap> _NewJuliaCFunc(JuliaEvalString("'a'"), fail);
 Error, NewJuliaCFunc: <arg_names> must be plain list
 
 #

diff --git a/JuliaInterface/tst/utils.tst b/JuliaInterface/tst/utils.tst
@@ -32,7 +32,7 @@ gap> JuliaSetVal(fail, 1);
 Error, JuliaSetVal: <name> must be a string
 gap> JuliaSetVal("foo", JuliaEvalString("1"));
 gap> JuliaGetGlobalVariable("foo");
-<Julia: 1>
+1
 
 ##
 gap> JuliaTuple([]);
-Original file line number
+Diff line change
@@ Expand Up / @@ -37,7 +37,7 @@ false @@
     ##
     gap> Julia.GAPRealCycModule.test_this_module();
-    <Julia: true>
+    true
     ##
     gap> STOP_TEST( "realcyc.tst", 1 );
@@ Expand Down @@