Skip to content
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

Fix #34316 #34817

Merged
merged 4 commits into from
Feb 21, 2020
Merged

Fix #34316 #34817

merged 4 commits into from
Feb 21, 2020

Conversation

SebastianM-C
Copy link
Contributor

The differences between the proposed solution in #34771 (comment) are

function hypot(x, y)
    #Return Inf if either or both inputs is Inf
    if isinf(x) || isinf(y)
        return promote_type(typeof(x), typeof(y))(Inf)
    end

    hypot(x, y)
end
  • remove the hypot definition from Furlongs. I looked at why the tests were passing with the previous definition of hypot and I discovered that the fallback was not actually tested because of the additional method.

Return Inf if either input is Inf
Create a testset for hypot and add tests in the unitless case.
base/math.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC added backport 1.4 bugfix This change fixes an existing bug labels Feb 19, 2020
base/math.jl Outdated Show resolved Hide resolved
base/math.jl Outdated Show resolved Hide resolved
Adapt code to work without promotion rules and remoeve float specific 
behavior
@SebastianM-C
Copy link
Contributor Author

Thanks for the review. I added the suggested changes in the latest commit.

@JeffBezanson JeffBezanson merged commit 8e3aa66 into JuliaLang:master Feb 21, 2020
@SebastianM-C SebastianM-C deleted the hypot branch February 21, 2020 17:48
KristofferC pushed a commit that referenced this pull request Feb 21, 2020
Fix hypot with both arguments 0

Remove hypot from Furlongs in order to test the defined fallback

(cherry picked from commit 8e3aa66)
@KristofferC KristofferC mentioned this pull request Feb 21, 2020
26 tasks
birm pushed a commit to birm/julia that referenced this pull request Feb 22, 2020
Fix hypot with both arguments 0

Remove hypot from Furlongs in order to test the defined fallback
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
Fix hypot with both arguments 0

Remove hypot from Furlongs in order to test the defined fallback
BioTurboNick pushed a commit to BioTurboNick/julia that referenced this pull request Apr 13, 2020
Fix hypot with both arguments 0

Remove hypot from Furlongs in order to test the defined fallback

(cherry picked from commit 8e3aa66)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants