Skip to content

perf: avoid test_inverse import-time compilation #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Beforerr wants to merge 1 commit into
base: master
Choose a base branch
from
Open

perf: avoid test_inverse import-time compilation #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions src/InverseFunctions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,14 @@ The function tests (as a `Test.@testset`) if
On Julia >= 1.9, you have to load the `Test` standard library to be able to use
this function.
"""
function test_inverse end
function test_inverse(args...; kwargs...)
length(args) == 2 || throw(MethodError(test_inverse, args))
throw(ArgumentError("InverseFunctions.test_inverse requires the Test standard library. Did you forget to load Test?"))
end
Comment on lines +29 to +32
Copy link
Copy Markdown
Member

@devmotion devmotion May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the behaviour: Currently, the error hint with the question is only shown when the extension is actually not loaded. This message here will instead be displayed unconditionally, and users will see an ArgumentError with an incorrect message instead of eg a MethodError.

Copy link
Copy Markdown
Author

@Beforerr Beforerr May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I changed this so the fallback no longer changes unrelated call errors.

The fallback is still a varargs method so the Test extension can add its real test_inverse(f, x; ...) method without overwriting during precompilation. But it now only emits the friendly missing-Test message for the normal two-argument call. Other signatures throw MethodError, matching existing behavior better.

Copy link
Copy Markdown
Member

@devmotion devmotion May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, even the behaviour of the new version is different from master. There's no point in asking the user to load the extension if it's already loaded.

Copy link
Copy Markdown
Author

@Beforerr Beforerr May 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If test is already loaded, the ArgumentError would never be throwed I think.

Since Julia owns MethodError display so to improve load-time compilation,
options are :

  1. Exact old exception behavior, faster load: MethodError, but no “Did you forget…” hint.
  2. Or friendly message, changed exception: define fallback method, throw ArgumentError / ErrorException.

I felt like keeping original UX with longer load-time is worse as this function is rarely used downstream and require Test to load first to use it anyway.


@static if !isdefined(Base, :get_extension)
include("../ext/InverseFunctionsDatesExt.jl")
include("../ext/InverseFunctionsTestExt.jl")
end

# Better error message if users forget to load Test
if isdefined(Base, :get_extension) && isdefined(Base.Experimental, :register_error_hint)
function __init__()
Base.Experimental.register_error_hint(MethodError) do io, exc, _, _
if exc.f === test_inverse &&
(Base.get_extension(InverseFunctions, :InverseFunctionsTest) === nothing)
print(io, "\nDid you forget to load Test?")
end
end
end
include("../ext/InverseFunctionsTestExt.jl")
end

end # module
32 changes: 32 additions & 0 deletions test/test_inverse.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,38 @@ end
InverseFunctions.test_inverse(inverse, log, compare = ===)
end

@static if isdefined(Base, :get_extension)
@testset "test_inverse without Test" begin
script = """
using InverseFunctions
try
InverseFunctions.test_inverse(identity, 1)
exit(1)
catch err
showerror(stdout, err)
exit(err isa ArgumentError ? 0 : 1)
end
"""
cmd = `$(Base.julia_cmd()) --startup-file=no --project=$(Base.active_project()) -e $script`
output = read(ignorestatus(cmd), String)
@test occursin("Did you forget to load Test?", output)

script = """
using InverseFunctions
try
InverseFunctions.test_inverse(identity)
exit(1)
catch err
showerror(stdout, err)
exit(err isa MethodError ? 0 : 1)
end
"""
cmd = `$(Base.julia_cmd()) --startup-file=no --project=$(Base.active_project()) -e $script`
output = read(ignorestatus(cmd), String)
@test occursin("MethodError", output)
end
end

@testset "maths" begin
InverseFunctions.test_inverse(!, false)

Expand Down
Loading