-
Notifications
You must be signed in to change notification settings - Fork 85
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
Test for things adding rules to DataType/UnionAll #563
Conversation
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.
Adding this test here will ensure that no such rule is added to ChainRules, but users writing their own rules could still break things for themselves. It might be helpful to add this to CRTU instead/as well?
test/global_state.jl
Outdated
end | ||
|
||
if function_type == DataType || function_type == UnionAll || function_type == Union | ||
@error "Bad constructor rrule. typeof(T)` not `Type{T}`" method |
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.
@error "Bad constructor rrule. typeof(T)` not `Type{T}`" method | |
@error "Bad constructor rrule. Use `Type{T}` not `typeof(T)`" method |
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.
Or Type{<:T}
probably?
Yeah, I guess we can put this into a function in CRTU and then just call it here |
Maybe |
This should let us catch errors like
#562
before they are merged.
idk if this is the best way, feedback appreciated.
The output this will show is:
This PR will fail until #562 is merged. We should merge one of these then rebase the other