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

"invalid subtyping" breaking change in 0.6 #20949

Closed
marius311 opened this issue Mar 8, 2017 · 11 comments
Closed

"invalid subtyping" breaking change in 0.6 #20949

marius311 opened this issue Mar 8, 2017 · 11 comments
Labels
doc This change adds or pertains to documentation types and dispatch Types, subtyping and method dispatch

Comments

@marius311
Copy link
Contributor

In 0.5 the following works,

abstract foo{T}
immutable bar <: foo end

but in 0.6 it gives "invalid subtyping in definition of bar". You can fix it by putting immutable bar <: foo{Any}, which afaict gives the behavior 0.5 used to.

If this is intended, unless I've missed it then I think this should be mentioned in NEWS.md. I'm probably biased because this pattern appeared in my code a lot, but I would vote for it being near the top.

Actually, FWIW I thought the 0.5 version is nicer, I have lots of types with many parameters, and its getting a bit ugly to keep writing foo{Any,Any,Any,...}, so actually unless I missed why this is required or this is actually a bug, my vote would be to allow the old syntax.

@yuyichao yuyichao added the doc This change adds or pertains to documentation label Mar 8, 2017
@JeffBezanson
Copy link
Sponsor Member

Agree this should be added to NEWS.

We can't automatically convert immutable bar <: foo to immutable bar <: foo{Any, Any, ...}, because we don't know if Any is a sensible parameter value for foo. If we allowed this again, the supertype of bar would be foo{T} where T<:Any, not foo{Any}.

Could you give a bit more realistic example, to get a sense of how this is used and whether it would be useful to support?

@tkelman
Copy link
Contributor

tkelman commented Mar 8, 2017

@JeffBezanson here are 41 examples from pkgeval (some may be duplicates due to dependencies) https://github.com/JuliaCI/pkg.julialang.org/search?utf8=✓&q=invalid+subtyping

@JeffBezanson
Copy link
Sponsor Member

What I really want to know is whether those 41 cases are desirable, or if they're just bugs. Is it good to be able to define a subtype of Distribution that doesn't specify its parameters, or is that more of an oversight?

@StefanKarpinski
Copy link
Sponsor Member

That seems pretty weird and likely to be a bug, imo, but the opinion of someone more familiar with Distributions would be helpful. @johnmyleswhite?

@marius311
Copy link
Contributor Author

In my case, I had some abstract type describing "linear operators" which could act on "vectors" in a certain basis. The abstract type was abstract LinearOp{Basis} and particular operators looked like immutable SomeOp <: LinearOp{SomeBasis}. The Basis parameter was used by a function which automatically converted input vectors to this basis before feeding them into the operator.

However, some operators could be coded up so they could act generically on a vector in any basis, in which case they'd be defined like immutable GenericOp <: LinearOp and bypass the automatic conversion function which was matching on specific Basis'es. Its this which I now need to write immutable GenericOp <: LinearOp{Any}.

In reality I had three such parameters so in a bunch of places <: LinearOp turned into <: LinearOp{Any,Any,Any} which just struck me as quite redundant looking. Anyway, I'll leave it to you guys that understand the intricacies of the type system to decide if this is anything worth having in 0.6.

@marius311
Copy link
Contributor Author

marius311 commented Mar 8, 2017

And I should probably mention that indeed even in 0.5 the supertype of bar in this case is not foo{Any} but a UnionAll, so probably my solution mentioned of in 0.6 replacing everything with <: foo{Any} is subtly different than what was happening in 0.5, although I haven't noticed a difference for my use-case.

@StefanKarpinski
Copy link
Sponsor Member

Thanks for the feedback, @marius311 – it's very helpful.

@JeffBezanson
Copy link
Sponsor Member

Yes, that's a helpful example.

Would some arrangement like this work:

abstract LinearOp

abstract LinearOpWithBasis{basis} <: LinearOp

struct GenericOp <: LinearOp ...

struct SomeOp <: LinearOpWithBasis{b} ...

A type like LinearOp{B} where B implies that there is some basis, you just don't know what it is, as opposed to a type that doesn't specify any particular basis. These are equivalent in many contexts, but this is the distinction the type system is trying to make here.

@ChrisRackauckas
Copy link
Member

I saw DiffEqJump and DiffEqBiological show up in the logs. Those are bugs: I meant for the abstract type to know a little bit of information to use for dispatching. I haven't dispatched off of it yet, so it didn't get caught. I like that the type system throws an error in this case now.

@marius311
Copy link
Contributor Author

A type like LinearOp{B} where B implies that there is some basis, you just don't know what it is, as opposed to a type that doesn't specify any particular basis. These are equivalent in many contexts, but this is the distinction the type system is trying to make here.

Thanks, yea, that makes sense. I think your suggestion would definitely work in my case.

abhinavd added a commit to abhinavd/Blink.jl that referenced this issue Dec 20, 2017
See JuliaLang/julia#20949
Got this warning during installation:
`WARNING: deprecated syntax "abstract Shell" at <>/.julia/v0.6/Blink/src/AtomShell/AtomShell.jl:6.
Use "abstract type Shell end" instead.`
@KristofferC
Copy link
Sponsor Member

Doesn't seem like any concrete thing left to do here for 0.7.

@JeffBezanson JeffBezanson added the types and dispatch Types, subtyping and method dispatch label Apr 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc This change adds or pertains to documentation types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

No branches or pull requests

7 participants