-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Conversation
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) |
Is this to emphasis that |
Ref: #8142 |
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. |
3482cdf
to
0efe98b
Compare
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 |
|
But the lower types aren't |
+1 for AbstractFloat and AbstractInteger |
I really dislike |
True. |
No. |
Can we not do this rename then? It seems unnecessary. |
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. |
Having multiple names for the same thing is always a bad idea. This is a pretty clear binary yes/no decision. |
To me part of the issue is that |
@jakebolewski We'll have to agree to disagree on that "always" statement. By that logic we should remove @JeffBezanson I understand your reasoning, but when I first started with julia it was not immediately obvious that |
I do agree that |
Am I mistaken that the reason for this is to avoid people mistakenly using abstract types as record fields ? What about making If you apply this to method arg tuples you can then distinguish between (don't mind me, I might just need more coffee) |
It might be useful to have Lint.jl warn about non-concrete record types, if it doesn't already. |
Would it make sense to have something like an |
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. |
Yes, I'm fine with this. @andreasnoack I agree we should have a type like this, to sit above AbstractFloat and FixedPoint. Maybe |
No big deal, but I'm wondering if you'd need the |
+1 for
As another possible name, in |
That's a good name but it sounds like it would also apply to |
👍 to |
|
Oh! That explains it. Thanks! (TLAs can be rather confusing at times, when you come from another domain) |
+1 that would be really good. |
Showing the type definition would be helpful (maybe without inner constructors) – this could quite easily be gotten via reflection. |
@StefanKarpinski Is there a way to access directly the actual type definition of |
@davidagold I guess you can just do what |
@yuyichao Ahah! I've found myself looking for something like that a number of times. Thank you! |
0efe98b
to
ef2038d
Compare
Rebased. |
There are changes to 3 binary files in contrib that probably shouldn't be here. |
Good catch, @tkelman. |
ef2038d
to
2e7dbd4
Compare
+1 from me too. Better do this sooner than later. |
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 |
There seems to be an unrelated issue with Travis tests running on legacy hardware. |
rename: FloatingPoint => AbstractFloat
I dispute the theory that names are either "cute" or "consistent". |
I didn't say cute isn't consistent. I said |
We really need to fix #11200 before removing these in 0.5. Deprecation without warnings pretty much defeats the purpose. |
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 |
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.
No description provided.