-
Notifications
You must be signed in to change notification settings - Fork 230
Add Aqua to tests #2257
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
Add Aqua to tests #2257
Conversation
a27bbd7 to
16e5ab7
Compare
| # TODO(mhauru) We skip testing for method ambiguities because it catches a lot of problems | ||
| # in dependencies. Would like to check it for just Turing.jl itself though. | ||
| Aqua.test_all(Turing; ambiguities=false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to test ambiguities, I suggest
| # TODO(mhauru) We skip testing for method ambiguities because it catches a lot of problems | |
| # in dependencies. Would like to check it for just Turing.jl itself though. | |
| Aqua.test_all(Turing; ambiguities=false) | |
| @testset "Aqua" begin | |
| # Test ambiguities separately without Base and Core | |
| # Ref: https://github.com/JuliaTesting/Aqua.jl/issues/77 | |
| Aqua.test_all(Turing; ambiguities = false) | |
| Aqua.test_ambiguities(Turing) | |
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks. I'll see if I can fix all the ambiguities, and if yes then commit this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I won't get to this today, and next week I'm off this project, so I'll merge what's here now and open an issue to come back to method ambiguities.
I tried out Aqua.jl and it caught some issues worth fixing, so I fixed them and made Aqua run as part of the test suite. Had to disable the method ambiguity checks though, because it was catching a lot of ambiguities not caused by us, but rather by our dependencies.
Ping @willtebbutt since he might be interested.