From 4ec53ab84b23a5d7efdeda6044a42c2a8871fa77 Mon Sep 17 00:00:00 2001 From: ThomasBreuer Date: Tue, 1 Oct 2024 16:22:50 +0200 Subject: [PATCH 1/2] 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 --- src/julia_to_gap.jl | 8 ++++++++ test/conversion.jl | 19 +++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/julia_to_gap.jl b/src/julia_to_gap.jl index 61a9a5ce1..53e3c0d7c 100644 --- a/src/julia_to_gap.jl +++ b/src/julia_to_gap.jl @@ -347,3 +347,11 @@ function GapObj_internal( end GAP.@install GapObj(func::Function) = WrapJuliaFunc(func) + +# For backwards compatibility, +# provide the generic methods for `julia_to_gap`. +# (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) = GapObj_internal(obj, nothing, Val(false)) +julia_to_gap(obj::Any; recursive::Bool) = julia_to_gap(obj) +julia_to_gap(obj::Any, recursion_dict::IdDict{Any,Any}; recursive::Bool = true) = julia_to_gap(obj) diff --git a/test/conversion.jl b/test/conversion.jl index 32af29a11..89d5e54cf 100644 --- a/test/conversion.jl +++ b/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,22 @@ end @test GAP.Globals.List(list, return_first_gap) == list end + @testset "Test julia_to_gap (backwards compatibility)" begin + @test GAP.julia_to_gap(27) == 27 + @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] + 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 + end end @testset "(Un)WrapJuliaFunc" begin From 4ffb34ccc8e033482d7b34ea6711b52756161bf3 Mon Sep 17 00:00:00 2001 From: ThomasBreuer Date: Tue, 8 Oct 2024 22:29:18 +0200 Subject: [PATCH 2/2] 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 | 8 ++--- src/gap_to_julia.jl | 2 +- src/julia_to_gap.jl | 14 ++++---- test/conversion.jl | 56 ++++++++++++++++++++++++++++++- 4 files changed, 66 insertions(+), 14 deletions(-) diff --git a/pkg/JuliaInterface/gap/convert.gd b/pkg/JuliaInterface/gap/convert.gd index 8949aa8b5..b158f00f6 100644 --- a/pkg/JuliaInterface/gap/convert.gd +++ b/pkg/JuliaInterface/gap/convert.gd @@ -120,7 +120,7 @@ #! immediate integer to Int64, #! #! -#! immediate FFE to the GapFFE &Julia; type, +#! immediate FFE to the FFE &Julia; type, #! #! #! &GAP; true to &Julia; true, @@ -147,7 +147,7 @@ #! otherwise to a &GAP; large integer, #! #! -#! GapFFE to immediate FFE, +#! FFE to immediate FFE, #! #! #! &Julia; true to &GAP; true, @@ -190,7 +190,7 @@ #! #! #! IsFFE and IsInternalRep to -#! GapFFE, +#! FFE, #! #! #! IsInt and IsSmallIntRep to @@ -308,7 +308,7 @@ #! #! #! -#! Int64, GapObj, GapFFE, and Bool +#! Int64, GapObj, FFE, and Bool #! #! automatic conversion #! diff --git a/src/gap_to_julia.jl b/src/gap_to_julia.jl index fef047cc7..b1bfbc20d 100644 --- a/src/gap_to_julia.jl +++ b/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 | diff --git a/src/julia_to_gap.jl b/src/julia_to_gap.jl index 53e3c0d7c..e8887c708 100644 --- a/src/julia_to_gap.jl +++ b/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 @@ -349,9 +347,9 @@ end GAP.@install GapObj(func::Function) = WrapJuliaFunc(func) # For backwards compatibility, -# provide the generic methods for `julia_to_gap`. +# 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) = GapObj_internal(obj, nothing, Val(false)) -julia_to_gap(obj::Any; recursive::Bool) = julia_to_gap(obj) -julia_to_gap(obj::Any, recursion_dict::IdDict{Any,Any}; recursive::Bool = true) = julia_to_gap(obj) +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)) diff --git a/test/conversion.jl b/test/conversion.jl index 89d5e54cf..423b7c095 100644 --- a/test/conversion.jl +++ b/test/conversion.jl @@ -555,11 +555,46 @@ end 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] + @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 @@ -569,6 +604,25 @@ end 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