-
-
Notifications
You must be signed in to change notification settings - Fork 117
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
Compat.isapprox method with nans keyword argument #309
Conversation
@@ -947,6 +947,16 @@ if VERSION < v"0.4.0-dev+6578" | |||
≉(x,y) = !(x ≈ y) | |||
export ≈, ≉ | |||
end | |||
if VERSION < v"0.6.0-dev.2093" # Compat.isapprox to allow for NaNs | |||
if VERSION >= v"0.4.0-dev+6578" |
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.
We require julia 0.4
, so this should always be true?
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.
Thanks. Then I might as well just remove the previous definition as part of this PR.
6af46c9
to
9ea34db
Compare
@@ -1774,4 +1761,13 @@ if VERSION >= v"0.5.0-dev+5509" && VERSION < v"0.6.0-dev.1614" | |||
include_string(".|(xs...) = broadcast(|, xs...)") | |||
end | |||
|
|||
if VERSION < v"0.6.0-dev.2093" # Compat.isapprox to allow for NaNs | |||
using Base.rtoldefault | |||
function isapprox(x::Number, y::Number; rtol::Real=rtoldefault(x,y), atol::Real=0, nans::Bool=false) |
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.
don't want to overwrite the default when nans is not specified, right?
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.
Yes. That was the plan.
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.
is it possible to define a kwarg method without a default value?
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.
Nope, that's a syntax error saying that the kwarg needs a default value.
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.
Let me phrase the original question differently then. Doesn't the default-value method here overwrite the base method without the nan kwarg?
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.
I'm not importing the Base method so I wouldn't think so.
Can we merge this? The tests are failing because of the |
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.
👍
* Remove `take!(::Task)` definition for Julia versions prior to 0.6 Was added in #307. * Remove `redirect_std*(f, stream)` definitions for Julia prior to v0.6 Were added in #275. * Remove at-__DIR__ macro definition for Julia versions prior to 0.6 Was added in #281. * Remove `broadcast` definition for equal-length tuples Was added in #324 and #328 * Remove definitions of `unsafe_get` and `isnull` fallsback Were added in #287. * Remove defintions of `xor` and `⊻` Were added in #289. * Definitions of `numerator` and `denominator` Were added in #290. * Remove defintion of `iszero` Was added in #305. * Remove definition of `>:` Was added in #336 * Remove definition of `take!(::Base.AbstractIOBuffer)` Was added in #290. * Remove definiton of `.&` and `.|` Were added in #306. * Remove definition of `Compat.isapprox` Was added in #309.
No description provided.