-
Notifications
You must be signed in to change notification settings - Fork 15
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 more Aqua tests by solving piracy and ambiguities issues #143
Conversation
Thank you, this is very much appreciated! A new breaking release is reasonable for the change you suggested. |
The SProjector warning in the doc builder is fixed in a separate PR. Thanks for coordinating all these fixes! I have noted down this bounty and I will be sending you more details on how it will be processed at the end of the month (it will probably be a slow process, as it is managed through a non-profit -- on your end, you will be asked to fill out a form with payment details, but that is all the boilerplate you need to worry about). |
Thanks @Krastanov for the help in passing CI tests. Do I need to write an email to you at some address to help processing the request? |
Yes, please just send along an email with your name and a link to the bounty. NumFocus will contact you directly to get the transfer information. |
Fixes #135
This PR is the main contribution to solve #135. But there are some others I will open in other packages, they are required to make everything work.
It removes all avoidable piracies and all direct ambiguities within the package. As a part of it I also addressed a TODO because otherwise some ambiguities were much difficult to solve: I changed all methods such as
observable(refs::Tuple{Vararg{RegRef, N}}, obs, something=nothing; time)
toobservable(refs::Tuple{Vararg{RegRef, N}}, obs; something=nothing, time=nothing)
, which means thatsomething
is now a keyword argument of those functions. This is a minor breaking change so maybe it will require a new major version if we want to be strict about it.