Skip to content

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented Dec 26, 2020

This tries to make the errors from this package just a hair less inscrutable.

Edit: closes #81

Copy link
Member

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except the think with isnothing

@oxinabox
Copy link
Member

Oh sorry I missed this before, but shouldn't we also error if rule is not defined for frule?

@mcabbott
Copy link
Member Author

Probably, I only fixed things which bit me, so far...

@mcabbott
Copy link
Member Author

I've added one for frule too now. (And I think I got the signature right!)

BTW I'm not entirely sure MethodError is the right thing here. It has the right information, but isn't quite the truth -- a method does exist, it's just the fallback one. Maybe an explicit description is clearer?

@nickrobinson251
Copy link
Contributor

closes #81

@oxinabox
Copy link
Member

oxinabox commented Dec 28, 2020

BTW I'm not entirely sure MethodError is the right thing here. It has the right information, but isn't quite the truth -- a method does exist, it's just the fallback one. Maybe an explicit description is clearer?

Manually thrown MethodErrors do that though.
E.g. https://github.com/JuliaMath/SpecialFunctions.jl/blob/master/src/gamma.jl#L528
I do agree sometimes they are confusing.
But the nice highlighting of what arguments are almost correct is I think worth it.

We can always change it again later

@oxinabox oxinabox merged commit 3436597 into JuliaDiff:master Dec 28, 2020
@mcabbott mcabbott deleted the err branch December 28, 2020 19:07
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.

directly report if no rrule/frule is defined
3 participants