-
Notifications
You must be signed in to change notification settings - Fork 219
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
Enable Aqua.test_ambiguities #2261
Comments
For context, here are the method ambiguities: https://github.com/TuringLang/Turing.jl/actions/runs/10161073643/job/28098796083?pr=2290#step:8:305 They arise from three functions:
Turing.jl/src/optimisation/Optimisation.jl Lines 278 to 286 in 07cc40b
I ran the optimisation test suite ( Restricting the second argument to these would make sense as a fix for the method ambiguity, and in all likelihood would capture reasonable use cases, but would technically be a breaking change. |
Would it help if we defined something like
? Not sure what the method should do. I guess it could call |
Having mulled it over a bit, I think my preferred option would just be to swallow the breaking change and bump the minor version, as otherwise we will end up playing whack-a-mole with any other dependencies that might choose to define If we think it's too trivial to release 0.34 over this, we could wait until there are other breaking changes to be made and then bundle them in together. (Most mature packages do that anyway.) And use github milestones ;) |
In #2257 I disabled method ambiguity checking, but @devmotion suggested enabling them with
We should do that and see if we can fix the 6 ambiguities that it warns about. I'm happy to do this, but off next week, so will take a moment.
The text was updated successfully, but these errors were encountered: