Skip to content

Conversation

@mhauru
Copy link
Member

@mhauru mhauru commented Jun 6, 2024

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.

@mhauru mhauru force-pushed the mhauru/code-formatting branch from a27bbd7 to 16e5ab7 Compare June 6, 2024 16:24
Comment on lines +6 to +8
# 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)
Copy link
Member

@devmotion devmotion Jun 6, 2024

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

Suggested change
# 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

Copy link
Member Author

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.

Copy link
Member Author

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.

@mhauru mhauru merged commit af812b8 into master Jun 7, 2024
@mhauru mhauru deleted the mhauru/code-formatting branch June 7, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants