Proper testing of extension error messages

Author: MilesCranmer

ext/DynamicExpressionsZygoteExt.jl

@@ -1,7 +1,7 @@
 module DynamicExpressionsZygoteExt
 
 import Zygote: gradient
-import DynamicExpressions.EvaluateEquationDerivativeModule: _zygote_gradient
+import DynamicExpressions.ExtensionInterfaceModule: _zygote_gradient
 
 function _zygote_gradient(op::F, ::Val{1}) where {F}
     function (x)

src/DynamicExpressions.jl

@@ -1,6 +1,7 @@
 module DynamicExpressions
 
 include("Utils.jl")
+include("ExtensionInterface.jl")
 include("OperatorEnum.jl")
 include("Equation.jl")
 include("EquationUtils.jl")
@@ -9,7 +10,6 @@ include("EvaluateEquationDerivative.jl")
 include("EvaluationHelpers.jl")
 include("SimplifyEquation.jl")
 include("OperatorEnumConstruction.jl")
-include("ExtensionInterface.jl")
 include("Random.jl")
 
 import PackageExtensionCompat: @require_extensions

src/EvaluateEquationDerivative.jl

@@ -5,15 +5,14 @@ import ..OperatorEnumModule: OperatorEnum
 import ..UtilsModule: is_bad_array, fill_similar
 import ..EquationUtilsModule: count_constants, index_constants, NodeIndex
 import ..EvaluateEquationModule: deg0_eval, get_nuna, get_nbin
+import ..ExtensionInterfaceModule: _zygote_gradient
 
 struct ResultOk2{A<:AbstractArray,B<:AbstractArray}
     x::A
     dx::B
     ok::Bool
 end
 
-_zygote_gradient(args...) = error("Please load the Zygote.jl package.")
-
 """
     eval_diff_tree_array(tree::AbstractExpressionNode{T}, cX::AbstractMatrix{T}, operators::OperatorEnum, direction::Integer; turbo::Bool=false)
 

src/ExtensionInterface.jl

@@ -1,14 +1,14 @@
 module ExtensionInterfaceModule
 
 function node_to_symbolic(args...; kws...)
-    return error(
-        "Please load the `SymbolicUtils` package to use `node_to_symbolic(::AbstractExpressionNode, ::AbstractOperatorEnum; kws...)`.",
-    )
+    return error("Please load the `SymbolicUtils` package to use `node_to_symbolic`.")
 end
 function symbolic_to_node(args...; kws...)
-    return error(
-        "Please load the `SymbolicUtils` package to use `symbolic_to_node(::Symbolic, ::AbstractOperatorEnum; kws...)`.",
-    )
+    return error("Please load the `SymbolicUtils` package to use `symbolic_to_node`.")
+end
+
+function _zygote_gradient(args...)
+    return error("Please load the Zygote.jl package.")
 end
 
 end

test/test_deprecations.jl

@@ -0,0 +1,20 @@
+using DynamicExpressions
+using Test
+using Zygote
+
+operators = OperatorEnum(; binary_operators=[+, -, *, /], unary_operators=[cos, sin])
+x1, x2 = Node{Float64}(; feature=1), Node{Float64}(; feature=2)
+tree = cos(2.1 * x1)
+
+# Also test warnings:
+for constructor in (OperatorEnum, GenericOperatorEnum)
+    operators = constructor(; binary_operators=[+, -, *, /], unary_operators=[cos, sin])
+    VERSION >= v"1.9" &&
+        @test_warn "The `tree(X; kws...)` syntax is deprecated" tree([1.0; 2.0;;])
+
+    constructor == GenericOperatorEnum && continue
+
+    tree'([1.0; 2.0;;])
+    VERSION >= v"1.9" &&
+        @test_warn "The `tree(X; kws...)` syntax is deprecated" tree'([1.0; 2.0;;])
+end

test/test_initial_errors.jl

@@ -1,34 +1,36 @@
 using DynamicExpressions
 using Test
-using Zygote
 
 # Before defining OperatorEnum, calling the implicit (deprecated)
 # syntax should fail:
-tree = Node(; feature=1)
+tree = Node{Float64}(; feature=1)
 
 if VERSION >= v"1.8"
-    @test_throws ErrorException tree([1.0 2.0]')
-    @test_throws "Please use the " tree([1.0 2.0]')
-    @test_throws ErrorException tree'([1.0 2.0]')
-    @test_throws "Please use the " tree'([1.0 2.0]')
+    @test_throws ErrorException tree([1.0; 2.0;;])
+    @test_throws "Please use the " tree([1.0; 2.0;;])
+    @test_throws ErrorException tree'([1.0; 2.0;;])
+    @test_throws "Please use the " tree'([1.0; 2.0;;])
 end
 
+# Initial strings are still somewhat useful
 @test string(tree) == "x1"
 @test string(Node(1, tree)) == "unary_operator[1](x1)"
 @test string(Node(1, tree, tree)) == "binary_operator[1](x1, x1)"
 
-# Also test warnings:
-for constructor in (OperatorEnum, GenericOperatorEnum)
-    operators = constructor(; binary_operators=[+, -, *, /], unary_operators=[cos, sin])
-    tree([1.0 2.0]')
-    # Can't test for this:
-    # expected_warn_msg = "The `tree(X; kws...)` syntax is deprecated"
-    # @test occursin(expected_warn_msg, msg)
+# Before loading extensions, should fail with helpful message:
+operators = OperatorEnum(; binary_operators=[+, -, *, /], unary_operators=[cos, sin])
+x1, x2 = Node{Float64}(; feature=1), Node{Float64}(; feature=2)
+tree = cos(2.1 * x1) + sin(x2)
 
-    constructor == GenericOperatorEnum && continue
+if VERSION >= v"1.9"
+    @test_throws(
+        "Please load the `SymbolicUtils` package to use `node_to_symbolic`.",
+        node_to_symbolic(tree, operators)
+    )
+    @test_throws(
+        "Please load the `SymbolicUtils` package to use `symbolic_to_node`.",
+        symbolic_to_node(tree, operators)
+    )
 
-    tree'([1.0 2.0]')
-    # Can't test for this:
-    # expected_warn_msg = "The `tree'(X; kws...)` syntax is deprecated"
-    # @test occursin(expected_warn_msg, msg)
+    @test_throws("Please load the Zygote.jl package.", tree'(ones(2, 10)))
 end

test/unittest.jl

@@ -8,6 +8,10 @@ end
     include("test_initial_errors.jl")
 end
 
+@safetestset "Test deprecations" begin
+    include("test_deprecations.jl")
+end
+
 @safetestset "Test tree construction and scoring" begin
     include("test_tree_construction.jl")
 end