for backward compatibility, reintroduce julia_to_gap (#1056)

* for backward compatibility, reintroduce `juliao_gap`

- provide generic methods that delegate to `GapObj_internal`
- add tests
- move an `end` of a `@testset` to a better place

* next iteration

- change the `julia_to_gap` methods such that `GapObj_internal`
  gets called with the same `recursive` value;
  this way, we get the same behaviour for `julia_to_gap` as before
  the switch to `GapObj_internal`,
- extend the tests of the backwards compatibility to the conversions
  promised in the documentation,
- fix the conversion rule for matrices:
  call the conversion of the row objects with the given `recursive` value
  in order to convert the matrix entries or not, as requested,
- and use the term `FFE` in the documentation, not `GapFFE`

pkg/JuliaInterface/gap/convert.gd

@@ -120,7 +120,7 @@
 #!    immediate integer to <C>Int64</C>,
 #!  </Item>
 #!  <Item>
-#!    immediate FFE to the <C>GapFFE</C> &Julia; type,
+#!    immediate FFE to the <C>FFE</C> &Julia; type,
 #!  </Item>
 #!  <Item>
 #!    &GAP; <K>true</K> to &Julia; <C>true</C>,
@@ -147,7 +147,7 @@
 #!    otherwise to a &GAP; large integer,
 #!  </Item>
 #!  <Item>
-#!    <C>GapFFE</C> to immediate FFE,
+#!    <C>FFE</C> to immediate FFE,
 #!  </Item>
 #!  <Item>
 #!    &Julia; <C>true</C> to &GAP; <K>true</K>,
@@ -190,7 +190,7 @@
 #!  </Item>
 #!  <Item>
 #!    <C>IsFFE and IsInternalRep</C> to
-#!    <C>GapFFE</C>,
+#!    <C>FFE</C>,
 #!  </Item>
 #!  <Item>
 #!    <C>IsInt and IsSmallIntRep</C> to
@@ -308,7 +308,7 @@
 #!    </Row>
 #!    <HorLine/>
 #!    <Row>
-#!      <Item><C>Int64</C>, <C>GapObj</C>, <C>GapFFE</C>, and <C>Bool</C></Item>
+#!      <Item><C>Int64</C>, <C>GapObj</C>, <C>FFE</C>, and <C>Bool</C></Item>
 #!      <Item></Item>
 #!      <Item>automatic conversion</Item>
 #!    </Row>

src/gap_to_julia.jl

@@ -72,7 +72,7 @@ The following `gap_to_julia` conversions are supported by GAP.jl.
 | GAP filter    | default Julia type       | other Julia types     |
 |---------------|--------------------------|-----------------------|
 | `IsInt`       | `BigInt`                 | `T <: Integer         |
-| `IsFFE`       | `GapFFE`                 |                       |
+| `IsFFE`       | `FFE`                    |                       |
 | `IsBool`      | `Bool`                   |                       |
 | `IsRat`       | `Rational{BigInt}`       | `Rational{T}          |
 | `IsFloat`     | `Float64`                | `T <: AbstractFloat   |

src/julia_to_gap.jl

@@ -53,7 +53,7 @@ The following `GapObj` conversions are supported by GAP.jl.
 | Julia type                           | GAP filter   |
 |--------------------------------------|--------------|
 | `Int8`, `Int16`, ..., `BigInt`       | `IsInt`      |
-| `GapFFE`                             | `IsFFE`      |
+| `FFE`                                | `IsFFE`      |
 | `Bool`                               | `IsBool`     |
 | `Rational{T}`                        | `IsRat`      |
 | `Float16`, `Float32`, `Float64`      | `IsFloat`    |
@@ -248,9 +248,7 @@ function GapObj_internal(
     recursion_dict = handle_recursion(obj, ret_val, rec, rec_dict)
 
     for i = 1:rows
-        # We need not distinguish between recursive or not
-        # because we are just now creating the "row objects" in Julia.
-        ret_val[i] = GapObj_internal(obj[i, :], recursion_dict, Val(true))
+        ret_val[i] = GapObj_internal(obj[i, :], recursion_dict, Val(recursive))
     end
     return ret_val
 end
@@ -347,3 +345,11 @@ function GapObj_internal(
 end
 
 GAP.@install GapObj(func::Function) = WrapJuliaFunc(func)
+
+# For backwards compatibility,
+# provide methods for `julia_to_gap` that cover the conversions promised
+# in the documentation.
+# (Installing other methods for `julia_to_gap` will not work in recursive
+# situations, only `GapObj_internal` methods can be used for that.)
+julia_to_gap(obj::Any; recursive::Bool = false) = GapObj_internal(obj, nothing, Val(recursive))
+julia_to_gap(obj::Any, recursion_dict::IdDict{Any,Any}; recursive::Bool = false) = GapObj_internal(obj, nothing, Val(recursive))

test/conversion.jl

@@ -285,11 +285,10 @@
     yy = GAP.gap_to_julia(Vector{Tuple{Int64}}, xx)
     @test [(1,)] == yy
     @test typeof(yy) == Vector{Tuple{Int64}}
-
+  end
 end
 
 @testset "conversion to GAP" begin
-  end
 
   @testset "Defaults" begin
     @test GapObj(true)
@@ -555,6 +554,76 @@ end
     @test GAP.Globals.List(list, return_first_gap) == list
   end
 
+  @testset "Test julia_to_gap (backwards compatibility)" begin
+    # integers
+    @test GAP.julia_to_gap(Int8(27)) == 27
+    @test GAP.julia_to_gap(27) == 27
+    @test GAP.julia_to_gap(BigInt(27)) == 27
+    # FFE
+    x = GAP.evalstr("Z(3)")
+    @test GAP.julia_to_gap(x) == x
+    # Bool
+    @test GAP.julia_to_gap(true) == true
+    # Rat
+    @test GAP.julia_to_gap(1//2) == GAP.evalstr("1/2")
+    # Float64
+    @test GAP.julia_to_gap(.1) == GAP.evalstr(".1")
+    # String
+    @test GAP.julia_to_gap("a") == GAP.evalstr("\"a\"")
+    # Symbol
+    @test GAP.julia_to_gap(:a) == GAP.evalstr("\"a\"")
+    # Char
+    @test GAP.julia_to_gap('a') == GAP.evalstr("'a'")
+    # UnitRange
+    @test GAP.julia_to_gap(1:5) == GAP.evalstr("[ 1 .. 5 ]")
+    # StepRange
+    @test GAP.julia_to_gap(1:2:5) == GAP.evalstr("[ 1, 3 .. 5 ]")
+    # Tuple
+    l = GAP.julia_to_gap((1,"a"))
+    @test GAP.Globals.IsList(l)
+    @test l[2] == "a"  # default conversion is non-recursive
+    l = GAP.julia_to_gap((1,"a"), recursive = true)
+    @test l[2] == GAP.evalstr("\"a\"")
+    # Vector{Bool}
+    v = GAP.julia_to_gap([true, false, true])
+    @test GAP.Globals.IsBlistRep(v)
+    # BitVector
+    v = GAP.julia_to_gap(BitVector((true, false, true)))
+    @test GAP.Globals.IsBlistRep(v)
+    # Vector
+    @test GAP.julia_to_gap([1, 2, 3, 4]) == GAP.evalstr("[ 1, 2, 3, 4 ]")
+    l = GAP.julia_to_gap([[1, 2], [3, 4]])
+    @test l isa GapObj
+    @test l[1] == [1, 2]  # default conversion is non-recursive
+    l = GAP.julia_to_gap([[1, 2], [3, 4]], recursive = true)
+    @test l isa GapObj
+    @test l[1] isa GapObj
+    v = [1, 2]
+    l = GAP.julia_to_gap([v, v])
+    @test l[1] === l[2]
+    l = GAP.julia_to_gap([v, v], recursive = true)
+    @test l[1] === l[2]
+    @test GAP.julia_to_gap([v, v], IdDict(), recursive = true) isa GapObj
+    # Matrix
+    m = GAP.julia_to_gap([1//2 2//3; 3//4 4//5])
+    @test GAP.Globals.IsTable(m)
+    @test m[1, 1] == 1//2  # default conversion is non-recursive
+    m = GAP.julia_to_gap([1//2 2//3; 3//4 4//5], recursive = true)
+    @test GAP.Globals.IsMatrix(m)
+    @test m[1, 1] isa GapObj
+    # Dict
+    d = GAP.julia_to_gap(Dict("a" => v, "b" => v))
+    @test d isa GapObj
+    @test d.a == v  # default conversion is non-recursive
+    d = GAP.julia_to_gap(Dict("a" => v, "b" => v), recursive = true)
+    @test d isa GapObj
+    @test d.a isa GapObj
+    @test d.a === d.b
+    @test GAP.julia_to_gap(Dict("a" => v, "b" => v), IdDict(), recursive = true) isa GapObj
+    # Function
+    @test GAP.Globals.IsFunction(GAP.julia_to_gap(sqrt))
+    @test !(GAP.julia_to_gap(sqrt) isa Function)
+  end
 end
 
 @testset "(Un)WrapJuliaFunc" begin