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

rename: FloatingPoint => AbstractFloat #12162

Merged
merged 2 commits into from
Jul 30, 2015
Merged

Conversation

StefanKarpinski
Copy link
Member

No description provided.

@vtjnash
Copy link
Member

vtjnash commented Jul 15, 2015

what's the rationale for doing such a massive breaking change? it'll be quite a hassle to rename this in all packages (although i see that the deprecation is in place https://github.com/JuliaLang/julia/pull/12162/files#diff-7904f4ddd9158030529e0ed5ee8707eeR658)

@yuyichao
Copy link
Contributor

Is this to emphasis that AbstractFloat is in fact "abstract"?

@jiahao
Copy link
Member

jiahao commented Jul 15, 2015

Ref: #8142

@StefanKarpinski
Copy link
Member Author

People are regularly confused by the name. Most recent example:

https://groups.google.com/forum/#!topic/julia-users/ih3xEucbcU8

I'm not 100% sold on this change, but I figured I might as well put it out there.

@tbreloff
Copy link

I think it makes sense, and not too hard for people to find/replace the name. If you're going down this road, you should also change Integer --> AbstractInt

@ScottPJones
Copy link
Contributor

String already went to AbstractString, so this sounds OK to me, but maybe AbstractInteger instead of AbstractInt? (it isn't AbstractStr, for example)

@quinnj
Copy link
Member

quinnj commented Jul 15, 2015

But the lower types aren't Integer64, so I'd be in favor of AbstractInt.

@dpsanders
Copy link
Contributor

+1 for AbstractFloat and AbstractInteger

@pao pao changed the title rename: FloatingPoint => AbtractFloat rename: FloatingPoint => AbstractFloat Jul 15, 2015
@JeffBezanson
Copy link
Member

I really dislike AbstractInteger. It is way too verbose, and this type is quite useful.

@dpsanders
Copy link
Contributor

True. AbstractInt, then.

@JeffBezanson
Copy link
Member

No. Integer stays.

@jakebolewski
Copy link
Member

Can we not do this rename then? It seems unnecessary.

@tbreloff
Copy link

Can we have a middle ground? What if AbstractInt and AbstractFloat are the actual types, and FloatingPoint, Integer, and AbstractInteger are type aliases? That could at least ease the pain of transition.

@jakebolewski
Copy link
Member

Having multiple names for the same thing is always a bad idea. This is a pretty clear binary yes/no decision.

@JeffBezanson
Copy link
Member

To me part of the issue is that Integer is a rather robust abstract concept, but "floating point" is not so much. Floating point is more of a concrete kind of number, but we don't have an exact definition of what it takes to be a FloatingPoint. Few languages (are there any others?) have such a type. So the name is less standard, and it might be worth calling extra attention to the fact that it's an abstract type. For example to many people "floating point" means Float32.

@tbreloff
Copy link

@jakebolewski We'll have to agree to disagree on that "always" statement. By that logic we should remove typealias as a keyword.

@JeffBezanson I understand your reasoning, but when I first started with julia it was not immediately obvious that Integer was abstract and Int was concrete. I was tripped up several times thinking I was effectively writing Int64 when I wrote Integer. I obviously got used to the concept, but it would have been very different if the types were Int vs AbstractInt

@jakebolewski
Copy link
Member

I do agree that Integer is more of a robust concept. I feel the struggle here is not with concrete/abstract notions of Integer or FloatingPoint but that user's anchor on the name to represent a concrete notion of value type in other languages. In this sense Integer and FloatingPoint have the same problem.

@carnaval
Copy link
Contributor

Am I mistaken that the reason for this is to avoid people mistakenly using abstract types as record fields ?

What about making a::A mean that A must be concrete so that a::Abstract is invalid (since the type would be empty). Then, you add, say, a ::< A which does what :: does now but stands out syntactically.

If you apply this to method arg tuples you can then distinguish between f{T}(::Vector{T},::T) which matches (Vector{Int},Int) but not (Vector{Integer},Int) whereas f{T}(::Vector{T},::<T) matches both. In a sense it could be a way to get the "some existentials are constrained to concrete types" rule explicit ?

(don't mind me, I might just need more coffee)

@malmaud
Copy link
Contributor

malmaud commented Jul 15, 2015

It might be useful to have Lint.jl warn about non-concrete record types, if it doesn't already.

@andreasnoack
Copy link
Member

Would it make sense to have something like an ApproximateNumber or NumberWithError (probably not the best names) instead of FloatingPoint. When deciding on whether the LU should pivot or not, I'm using T<:FloatingPoint but this is not the right choice for FixedPoints so having a common abstract type would help in that case.

@kshyatt kshyatt added breaking This change will break code types and dispatch Types, subtyping and method dispatch labels Jul 15, 2015
@StefanKarpinski
Copy link
Member Author

Maybe because I'm the one who made this PR but I've already found myself using AbstractFloat in place of FloatingPoint. I just this it's so much better. On the other hand, I'm not in favor of AbstractInteger. So basically, I favor merging this PR and leaving it at that.

@JeffBezanson
Copy link
Member

Yes, I'm fine with this.

@andreasnoack I agree we should have a type like this, to sit above AbstractFloat and FixedPoint. Maybe RoundedReal?

@andreasnoack
Copy link
Member

No big deal, but I'm wondering if you'd need the AbstractFloat if you had RoundedReal. I'd expect that it is mainly rounded property more than the floating point property that is important for dispatch.

@timholy
Copy link
Member

timholy commented Jul 17, 2015

+1 for AbstractFloat from me.

a type like this, to sit above AbstractFloat and FixedPoint

As another possible name, in Color.jl we currently use the term Fractional.

@JeffBezanson
Copy link
Member

That's a good name but it sounds like it would also apply to Rational. I suspect the key property to dispatch on is whether results of operations will be rounded.

@ScottPJones
Copy link
Contributor

👍 to AbstractFloat, it follows the pattern already established of AbstractString, AbstractVector, AbstractArray, AbstractRNG (why wasn't that AbstractRange?) etc.
Patterns like that make it easier for normal humans to remember.
Fractional seems misleading to me, because you can also use floating point types just to represent integers, including very big integers that don't fit in 64-bits.
Somewhat OT: it would be nice if the ? help system showed if a type were abstract or not, it shows if a type has subtypes, which implies that it is abstract, but if the abstract type has no subtypes, you can't tell.
That might reduce the potential confusion with abstract type names such as Integer and Unsigned that people don't want to expand to AbstractInteger etc.

@quinnj
Copy link
Member

quinnj commented Jul 17, 2015

AbstractRNG => Abstract Random Number Generator

@ScottPJones
Copy link
Contributor

Oh! That explains it. Thanks! (TLAs can be rather confusing at times, when you come from another domain)

@JeffBezanson
Copy link
Member

the ? help system showed if a type were abstract or not

+1 that would be really good.

@StefanKarpinski
Copy link
Member Author

Showing the type definition would be helpful (maybe without inner constructors) – this could quite easily be gotten via reflection.

@davidagold
Copy link
Contributor

@StefanKarpinski Is there a way to access directly the actual type definition of Foo using reflection, or would one have to reconstruct it from, say Foo.super, Foo.parameters, Foo.mutable, Foo.abstract, fieldnames(Foo), etc?

@yuyichao
Copy link
Contributor

@davidagold I guess you can just do what xdump does.

@davidagold
Copy link
Contributor

@yuyichao Ahah! I've found myself looking for something like that a number of times. Thank you!

@JeffBezanson
Copy link
Member

Rebased.

@tkelman
Copy link
Contributor

tkelman commented Jul 29, 2015

There are changes to 3 binary files in contrib that probably shouldn't be here.

@StefanKarpinski
Copy link
Member Author

Good catch, @tkelman.

@nalimilan
Copy link
Member

+1 from me too. Better do this sooner than later.

@ivarne
Copy link
Member

ivarne commented Jul 30, 2015

One of the things that appealed to me when learning Julia, was the cute names for the abstract types. Unfortunately they cause trouble for beginners, and the performance characteristics of Julia makes it rather important to know if a type is abstract.

I don't like this PR, because it doesn't actually make Julia easier to learn, but just pushes the issue on to Integer and Range. We should either go all in on abstract Abstract***, and add the prefix to the style guide for naming abstract types, or continue the original plan with cute names. Renaming one type at the time (when a julia-users is confused) is just annoying.

@StefanKarpinski
Copy link
Member Author

There seems to be an unrelated issue with Travis tests running on legacy hardware.

StefanKarpinski added a commit that referenced this pull request Jul 30, 2015
rename: FloatingPoint => AbstractFloat
@StefanKarpinski StefanKarpinski merged commit f172f4e into master Jul 30, 2015
@StefanKarpinski StefanKarpinski deleted the sk/AbstractFloat branch July 30, 2015 21:35
@JeffBezanson
Copy link
Member

I dispute the theory that names are either "cute" or "consistent".

@ivarne
Copy link
Member

ivarne commented Jul 30, 2015

I didn't say cute isn't consistent. I said abstract Abstract*** isn't cute, and transitioning to non-cute names, but keeping a few cute ones, isn't consistent.

@tkelman
Copy link
Contributor

tkelman commented Jul 31, 2015

We really need to fix #11200 before removing these in 0.5. Deprecation without warnings pretty much defeats the purpose.

@tkelman
Copy link
Contributor

tkelman commented Jul 31, 2015

There seems to be an unrelated issue with Travis tests running on legacy hardware.

The issue on OSX looked like an unrelated timeout, on 64 bit Linux was a pollfd failure (we've probably got a bug lingering). The message about "legacy infrastructure" is going to be there for as long as we need to use sudo on Travis. They're trying to move people to use the Docker-based workers, which can't use sudo, you can only apt-get install packages from a whitelist which the juliadeps PPA isn't on. We could instead try doing it the way we do on appveyor and download a Julia nightly and use the big deps from there (they don't have separate libraries or headers for LLVM though), but that's a fair amount of work that no one has done yet. The Docker workers do have less of a queue time and let you cache things which could be useful, so maybe it's worth figuring out eventually. For now, ignore that message.

fatestigma added a commit to fatestigma/MonthOfJulia that referenced this pull request Dec 10, 2016
1. function Base.* must be explicitly imported to be extended
2. Update for Julia 5.0.
3.  has beed renamed to : JuliaLang/julia#12162
4. Int64 is not subclass of Unsigned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.