-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
add Pkg3 configs & fix DomainError for v1.0 #28
add Pkg3 configs & fix DomainError for v1.0 #28
Conversation
I don’t think we want to add Project.toml etc. There is scheduled to be an automated process where these files are added for all registered packages. Can you trim the PR to just the DomainError fix? |
Good? |
src/QuadGK.jl
Outdated
@@ -78,7 +78,11 @@ function evalrule(f, a,b, x,w,gw, nrm) | |||
Ig *= s | |||
E = nrm(Ik - Ig) | |||
if isnan(E) || isinf(E) | |||
throw(DomainError()) | |||
@static if VERSION < v"0.7.0" |
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.
The if
statement is not needed, since QuadGK master requires 0.7, and even if it didn't the 1-arg form of DomainError
is supported by Compat.jl
src/QuadGK.jl
Outdated
@static if VERSION < v"0.7.0" | ||
throw(DomainError()) | ||
else | ||
throw(DomainError(E)) |
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.
E
is the wrong argument here. We are really supposed to pass the x
that triggered the domain error. In this case I would pass the midpoint a + s
since it is impractical to go back and figure out exactly which x
it was. Also, DomainError
takes a second argument, a message, which we might as well take advantage of, e.g. DomainError(a+s, "integrand produced $E in the interval ($a, $b)")
DomainError
is deprecated in v1.0.Pkg3
and also enable CI for 1.0