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

hypot behaves like L2-norm, adjust the docstring to reflect this #31947

Merged
merged 1 commit into from
May 28, 2019

Conversation

giordano
Copy link
Contributor

@giordano giordano commented May 6, 2019

Fix #31941. Here it was agreed that the current behaviour of hypot is correct, but the docstring didn't match it.

I also took the occasion to add a bunch of tests for hypot. They've been shamelessly copied from #30301.

test/math.jl Outdated Show resolved Hide resolved
@giordano
Copy link
Contributor Author

giordano commented May 18, 2019

Bump 😃

@simonbyrne
Copy link
Contributor

I’m not at my computer at the moment: what’s the issue with all the broken tests?

@giordano
Copy link
Contributor Author

AppVeyor on x86_64 failed to download a file, buildbot on win32 has an sha512sum hash mismatch in another download. I don't think they're related to the new tests added by this PR.

@simonbyrne
Copy link
Contributor

Sorry, I meant all the new @test_brokens?

@giordano
Copy link
Contributor Author

giordano commented May 19, 2019

Ah, they fail because vararg hypot is completely broken: #27141 😃 I've copied tests from #30301, marking as broken those not passing

@simonbyrne
Copy link
Contributor

I'm not a huge fan of bundling the unrelated tests with the docstring changes. Can you move them to a separate PR? Also, a lot of the @inferreds are probably overkill.

@giordano
Copy link
Contributor Author

I've completely removed the tests from this PR, only the docstring change is here

@giordano
Copy link
Contributor Author

Bump 🙂

@simonbyrne simonbyrne merged commit 93972d1 into JuliaLang:master May 28, 2019
@giordano giordano deleted the hypot-docstring branch May 28, 2019 18:28
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.

hypot with complex arguments
4 participants