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.

Sign up for GitHub

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

Jump to bottom

Add missing methods to round #30456

Closed
wants to merge 2 commits into from
Closed

Add missing methods to round #30456

wants to merge 2 commits into from

Conversation

sam0410
Copy link
Contributor

@sam0410 sam0410 commented Dec 19, 2018

Fixes issue #24021

ararslan
ararslan reviewed Dec 19, 2018
View reviewed changes
base/complex.jl Outdated Show resolved Hide resolved
ararslan
ararslan reviewed Dec 19, 2018
View reviewed changes
test/complex.jl Outdated Show resolved Hide resolved
@sam0410
Copy link
Contributor Author

sam0410 commented Dec 19, 2018 •
edited
Loading

Thanks for the quick review @ararslan. I have made the changes.

ararslan
ararslan reviewed Dec 19, 2018
View reviewed changes
test/complex.jl Outdated Show resolved Hide resolved
ararslan
ararslan previously approved these changes Dec 19, 2018
View reviewed changes
@ararslan ararslan added the maths Mathematical functions label Dec 19, 2018
@ararslan ararslan requested a review from simonbyrne December 19, 2018 23:32
@ararslan ararslan dismissed their stale review December 20, 2018 06:04

This appears to cause method ambiguities

@sam0410
Copy link
Contributor Author

sam0410 commented Dec 20, 2018

I fixed the ambiguities @ararslan. Please review.

stevengj
stevengj reviewed Dec 24, 2018
View reviewed changes
base/float.jl Outdated Show resolved Hide resolved
stevengj
stevengj reviewed Dec 24, 2018
View reviewed changes
base/complex.jl Outdated Show resolved Hide resolved
stevengj
stevengj reviewed Dec 24, 2018
View reviewed changes
base/missing.jl Outdated Show resolved Hide resolved
simonbyrne
simonbyrne reviewed Jan 3, 2019
View reviewed changes
base/float.jl Outdated Show resolved Hide resolved
@StefanKarpinski StefanKarpinski added backport 1.1 triage This should be discussed on a triage call and removed backport 1.1 labels Jan 31, 2019
@Keno Keno removed backport 1.1 triage This should be discussed on a triage call labels Mar 14, 2019
@Keno
Copy link
Member

Keno commented Mar 14, 2019

Triage: Fine change, no backport.

@BeastyBlacksmith BeastyBlacksmith mentioned this pull request Jun 20, 2019
Closed
@oscardssmith
Copy link
Member

This is still broken. Should we rebase?

@stevengj
Copy link
Member

stevengj commented Apr 6, 2021

Rebased.

@StefanKarpinski
Copy link
Member

This seems to have gotten caught with a bum CI run. Give it another try since CI is mostly better now?

@stevengj
Copy link
Member

I'm still getting a ERROR: The working directory is dirty in the buildbot/package_linux64 tests, which looks unrelated.

@stevengj
Copy link
Member

Looks like there is a method ambiguity:

Error in testset ambiguous:
Test Failed at /Users/julia/buildbot/worker-tabularasa/tester_macos64/build/share/julia/test/ambiguous.jl:157
  Expression: isempty(ambig)
   Evaluated: isempty(Set(Any[(Tuple{typeof(round), Type{Complex{T}}, Real} where T<:Real, Tuple{typeof(round), Type{T}, Rational{Tr}} where {T, Tr}), (Tuple{typeof(round), Type{T}, Real} where T<:Real, Tuple{typeof(round), Type{T}, Rational{Tr}} where {T, Tr})]))
Error in testset ambiguous:
Test Failed at /Users/julia/buil

@brenhinkeller brenhinkeller added the bugfix This change fixes an existing bug label Jul 25, 2022
@brenhinkeller
Copy link
Contributor

Looks like this still needs doing 😅. Still has method ambiguities?

@stevengj
Copy link
Member

AFAIK, yes. Should just need a little loving care to get merged.

@StefanKarpinski
Copy link
Member

Bump...

@ararslan
Copy link
Member

ararslan commented Sep 7, 2023

@oscardssmith, can you comment on why this was closed?

@oscardssmith
Copy link
Member

#50812 adds this functionality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevengj stevengj stevengj left review comments

@simonbyrne simonbyrne simonbyrne left review comments

@ararslan ararslan ararslan left review comments

Assignees
No one assigned
Labels
bugfix This change fixes an existing bug maths Mathematical functions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@sam0410 @Keno @oscardssmith @stevengj @StefanKarpinski @brenhinkeller @ararslan @simonbyrne @ViralBShah