Skip to content
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.

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

more informative @test x ≈ y failure message #55812

Open
stevengj opened this issue Sep 19, 2024 · 3 comments
Open

more informative @test x ≈ y failure message #55812

stevengj opened this issue Sep 19, 2024 · 3 comments
Labels
stdlib Julia's standard library testsystem The unit testing framework and Test stdlib

Comments

@stevengj
Copy link
Member

stevengj commented Sep 19, 2024

It would be nice to have a more informative failure message for a @test x ≈ y that:

  1. If one of the operands iszero, and the caller didn't pass a explicit atol, warn them that they might want to specify an atol (or rewrite e.g. x - y ≈ 0 to x ≈ y), and direct them to the isapprox docs.
  2. If it failed a relative tolerance (rtol) check, print out the computed relative error norm(x - y) / max(norm(x), norm(y)). See also report error magnitude on @test x ≈ y failure #55613

(I feel like this has been raised before, but I can't find an issue?)

@stevengj stevengj added the testsystem The unit testing framework and Test stdlib label Sep 19, 2024
@stevengj stevengj changed the title more informative @test x ≈ y error message more informative @test x ≈ y failure message Sep 19, 2024
@nsajko nsajko added the stdlib Julia's standard library label Sep 19, 2024
@barucden
Copy link
Contributor

barucden commented Sep 19, 2024

I think the first point here is a new one, otherwise: #55613

Anyway, how should one implement it without duplicating the logic of isapprox? Or how to avoid making PkgTest depend on LinearAlgebra for norm of vectors?

@stevengj
Copy link
Member Author

stevengj commented Sep 19, 2024

I think you have to duplicate some of the logic. Fortunately that logic is very simple.

Yes, that would make Test (not Pkg) explicitly depend on LinearAlgebra, but I don't see that as a big deal. LinearAlgebra is already implicitly loaded inside Julia, and is used when you call isapprox whether or not you explicitly depend on it.

(Or we could add a Base.isapprox_showerror(io, x, y; kws...) function, and then add additional methods in LinearAlgebra for arrays, then call this from Test. This would have the advantage of being easier to extend for more types. It would still involve some duplicated logic, though.)

@nsajko
Copy link
Contributor

nsajko commented Sep 19, 2024

LinearAlgebra is already implicitly loaded inside Julia

But that situation isn't ideal: #51633

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Julia's standard library testsystem The unit testing framework and Test stdlib
Projects
None yet
Development

No branches or pull requests

3 participants