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

make Irrational <: AbstractIrrational #24245

Merged
merged 1 commit into from
Oct 26, 2017

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Oct 20, 2017

As I mentioned in #24238, adding new irrational types, e.g. to represent exact multiples of π, would be vastly easier if we had Irrational <: AbstractIrrational <: Real rather than Irrational <: Real, because most of the methods for promotion etc. would be shared with the existing Irrational type.

This is a pretty simple change that I think we should make immediately.

@stevengj stevengj added maths Mathematical functions triage This should be discussed on a triage call labels Oct 20, 2017
@nalimilan
Copy link
Member

Can you explain why subtyping Irrational doesn't work? That's not clear to me even after reading the issue.

@stevengj
Copy link
Member Author

@nalimilan, what do you mean? Our current Irrational is not an abstract type, so it is impossible to subtype.

"""
struct Irrational{sym} <: Real end
abstract type AbstractIrrational <: Real end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be added to the "Abstract number types" section of doc/src/stdlib/numbers.md

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, done.

@stevengj
Copy link
Member Author

Travis failure seems unrelated?

The command "make -C moreutils mispipe" exited with 0.
0.08s$ make $BUILDOPTS -C base version_git.jl.phony
make: Entering directory `/home/travis/build/JuliaLang/julia/base'
sh /home/travis/build/JuliaLang/julia/base/version_git.sh /home/travis/build/JuliaLang/julia/base > version_git.jl.phony
fatal: bad revision '^^d6638e8434659c80ee967a60eb0858b1ec5af20'

@fredrikekre
Copy link
Member

Should be unrelated. I wonder why AppVeyor did not run on this?

@stevengj
Copy link
Member Author

Another case where I would like to use this: to define an irrational type with different promotion rules for the ChangePrecision package (see https://discourse.julialang.org/t/change-default-precision/6520/51?u=stevengj).

@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Oct 26, 2017
@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Oct 26, 2017
@StefanKarpinski
Copy link
Member

This ready to merge, @stevengj?

@JeffBezanson JeffBezanson merged commit 547c1aa into JuliaLang:master Oct 26, 2017
@stevengj stevengj deleted the abstractirrational branch October 26, 2017 19:39
@Jutho
Copy link
Contributor

Jutho commented Nov 3, 2017

What would now be the minimal interface/set of methods that a new subtype of AbstractIrrational needs to implement? Conversion to Float64 and BigFloat? I have a use case for square roots of rational numbers, which often occur in physical constants (e.g. Wigner symbols in my use case).

@garrison
Copy link
Member

I created a new issue for "Next steps/guidelines for AbstractIrrational" (#26550)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants