Use new inner constructor and type declaration syntax #107

andreasnoack · 2017-02-17T20:05:40Z

The new syntax for inner constructors doesn't parse on 0.5 so it will be ugly and more demanding to support on both versions. Furthermore, 0.6 will be out soon and nobody will care about 0.5. If they care anyway and we add features then we can backport those features to a 0.5 branch.

andyferris · 2017-02-19T01:35:45Z

LGTM. Thanks Andreas.

Just so you know, I was planning on merging and releasing #105 first, before merging this one. This plus major changes due to #106 are required before this package is usable on v0.6. Those other changes are backward compatible so maybe they should be released for v0.5 too (otherwise it would block backporting).

Would a @static if VERSION < ... version of this PR be a sensible approach? Or is it better all-round to focus development on a single version of Julia?

andyferris · 2017-02-19T01:37:32Z

CI is complaint about TupleN

andreasnoack · 2017-02-19T01:42:05Z

The problem is that the syntax doesn't parse at all so we'll end up with a lot of definitions like https://github.com/Evizero/UnicodePlots.jl/blob/master/src/graphics/bargraphics.jl#L1-L44 which I think is ugly. Personally, I think it is better to focus development hours on 0.6 and thereafter instead of compatibility.

There is a problem with the build bots so CI is running with old binaries that don't have the latest syntax changes. Tests pass locally on latest master.

c42f · 2017-02-20T04:54:36Z

I think merging this might be a bit premature - are the change here required to make the package work on 0.6, or are they syntactic niceties? If possible it would be nice for master to continue working with 0.5 for a while.

andreasnoack · 2017-02-20T11:41:13Z

The inner constructor syntax is deprecated so it gives warnings on master. The only way to make it work across 0.5 and 0.6 is to write that code as strings and use include_string conditionally on the version such as in the link I posted above.

timholy · 2017-02-20T11:59:17Z

There is always the (::Type{MyNewType{T}}){T}(arg) = new{T}(arg) syntax, which works on both.

andreasnoack · 2017-02-20T12:07:40Z

Thanks for pointing that out. I wasn't aware of that. I'll update the PR.

coveralls · 2017-02-20T15:51:24Z

Coverage decreased (-0.05%) to 67.452% when pulling a33600e on andreasnoack:master into 1be47cc on JuliaArrays:master.

andyferris · 2017-02-20T23:13:41Z

Thanks Andreas (and good idea, Tim).

I still think it would be better to delay this for just the moment and first address #106, which completely and utterly destroys any custom subtypes of StaticArray, is a much larger amount of work, and the fix is compatible with Julia v0.5.

At that point we can base development on v0.6 (and backport crucial fixes to v0.5 - but not fixing #106 first will make any backports less trivial). This will let us (in no particular order):

Not require Compat.
Make use of where syntax to simplify dispatch (e.g. this).
Let us take advantage of compiler (specifically, inference and constant propagation) improvements (this will mean, less @generated functions).
Keep code cleaner.

Basically, this package is unusually low-level and reliant on certain compiler behavior - I'd rather not have this package be artificially limited by continuing to support two versions of Julia.

I do apologize about the deprecation warning noise between now and then - I would suggest to keep rebasing this branch on the other changes for the next little while, and use this branch for downstream development work targeting v0.6 in the meantime.

I am also open to dropping v0.5 support now if people think this is a better route (how many backports will there really be?). But please, do keep in mind how completely broken custom static arrays are - deprecation warnings might be a good warning-sign for users :)

Based on JuliaArrays/StaticArrays.jl#107 (comment).

Based on JuliaArrays/StaticArrays.jl#107 (comment). fix mistake in TreeVertex constructor

Based on JuliaArrays/StaticArrays.jl#107 (comment). fix mistake in TreeVertex constructor Fix MechanismState constructor

andreasnoack · 2017-02-21T16:26:17Z

I don't really understand why merging these small deprecation warning fixes could be a problem for fixing custom static arrays. Notice, that the PR doesn't drop 0.5 anymore.

andyferris · 2017-02-22T00:34:49Z

I suppose it should be OK now that I'm tagging the similar_type changes. I'll be ripping out Compat sometime soon when we go to v0.6-only.

@andreasnoack this PR now has conflicts (my fault)

consequently drop 0.5 support.

c42f · 2017-02-22T01:23:56Z

Nice, thanks for putting up with 0.5 for a little longer :-)

andyferris · 2017-02-22T01:27:36Z

test/fixed_size_arrays.jl

-typealias Mat2d Mat{2,2, Float64, 4}
-typealias Mat3d Mat{3,3, Float64, 9}
-typealias Mat4d Mat{4,4, Float64, 16}
+@compat const Vec1d = Vec{1, Float64}


The sometimes const and sometimes not syntax isn't really great (not your fault, mind you).

Ah yes, I noticed this syntax quirk too. Hard to know what to do about it. In packages, it would be nice if all bindings at global scope were implicitly const by default. But for simple scripting that would be fairly horrible.

Could be a special property of Main? (Or some property that could be set on a module-by-module basis).

andyferris · 2017-02-22T23:39:05Z

Travis errors are odd...

andyferris · 2017-02-23T00:35:00Z

src/SMatrix.jl

@@ -66,7 +66,7 @@ end
        SMatrix{S1, S2, $T, L}(x)
    end
 end
-typealias SMatrixNoType{S1, S2, L, T} SMatrix{S1, S2, T, L}
+@compat SMatrixNoType{S1, S2, L, T} = SMatrix{S1, S2, T, L}


I don't know why, but I don't remember this existing, which is why I couldn't fathom the test failure... In theory, MMatrix should be symmetric. But don't worry about this for this PR.

andreasnoack · 2017-02-23T00:38:39Z

On 0.6, there seems to be an issue with @inferred and dot broadcast syntax. Looking into it now.

andyferris · 2017-02-23T00:49:05Z

I made an issue for that

andyferris · 2017-02-23T00:49:35Z

JuliaLang/julia#20620

andreasnoack · 2017-02-23T00:52:39Z

Oh, I missed that one. ~~I think there is an easy fix.~~ This was the first time I've looked at the dot syntax expression representation. A fix is more complicated than anticipated.

coveralls · 2017-02-23T02:59:20Z

Coverage increased (+0.03%) to 71.417% when pulling 4f578ff on andreasnoack:master into e6d7387 on JuliaArrays:master.

andyferris · 2017-02-23T04:58:41Z

I just merged this, Andreas. I still plan to remove Compat and v0.5 support sometime soonish.

andreasnoack · 2017-02-23T16:00:01Z

Could you make a release first?

c42f · 2017-02-27T03:07:28Z

Yes, let's make a release before we break everything with #113 (I guess this will end up dropping 0.5 support... honestly I was hoping to last a little longer!)

andyferris · 2017-02-27T03:41:30Z

I was wondering who the target audience was? There is no functional changes for v0.5 users and v0.6 users will be using quite a different codebase soon... @andreasnoack is using master for v0.6 development reasonable for the time-being?

andreasnoack · 2017-02-27T03:48:35Z

Using a release is slightly simpler in a larger group of collaborators but the main reason is Travis testing where it is pretty inconvenient to use master.

andyferris · 2017-02-27T04:25:54Z

Yes, Travis. That makes sense.

andreasnoack force-pushed the master branch 2 times, most recently from d340f7d to a33600e Compare February 20, 2017 15:11

tkoolen added a commit to JuliaRobotics/RigidBodyDynamics.jl that referenced this pull request Feb 21, 2017

Fix inner constructor deprecation warnings.

e4a7780

Based on JuliaArrays/StaticArrays.jl#107 (comment).

tkoolen added a commit to JuliaRobotics/RigidBodyDynamics.jl that referenced this pull request Feb 21, 2017

Fix inner constructor deprecation warnings.

6a197be

Based on JuliaArrays/StaticArrays.jl#107 (comment).

tkoolen added a commit to JuliaRobotics/RigidBodyDynamics.jl that referenced this pull request Feb 21, 2017

Fix inner constructor deprecation warnings.

3b12c91

Based on JuliaArrays/StaticArrays.jl#107 (comment). fix mistake in TreeVertex constructor

tkoolen added a commit to JuliaRobotics/RigidBodyDynamics.jl that referenced this pull request Feb 21, 2017

Fix inner constructor deprecation warnings.

a7ab829

Based on JuliaArrays/StaticArrays.jl#107 (comment). fix mistake in TreeVertex constructor Fix MechanismState constructor

tkoolen added a commit to JuliaRobotics/RigidBodyDynamics.jl that referenced this pull request Feb 21, 2017

Fix inner constructor deprecation warnings.

c0c89a6

Based on JuliaArrays/StaticArrays.jl#107 (comment). fix mistake in TreeVertex constructor Fix MechanismState constructor

tkoolen added a commit to JuliaRobotics/RigidBodyDynamics.jl that referenced this pull request Feb 21, 2017

Fix inner constructor deprecation warnings.

5f182d2

Based on JuliaArrays/StaticArrays.jl#107 (comment). fix mistake in TreeVertex constructor Fix MechanismState constructor

andreasnoack added 2 commits February 21, 2017 19:46

Use new inner constructor and type declaration syntax and

8f8f760

consequently drop 0.5 support.

Update for index traints changes

ed953ea

andreasnoack force-pushed the master branch from a33600e to ed953ea Compare February 22, 2017 00:47

andyferris reviewed Feb 22, 2017

View reviewed changes

andreasnoack changed the title ~~Use new inner constructor and type declaration syntax and consequently drop 0.5 support.~~ Use new inner constructor and type declaration syntax Feb 23, 2017

Make it work on 0.5

4f578ff

andyferris reviewed Feb 23, 2017

View reviewed changes

andyferris merged commit e24e1d8 into JuliaArrays:master Feb 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use new inner constructor and type declaration syntax #107

Use new inner constructor and type declaration syntax #107

andreasnoack commented Feb 17, 2017

andyferris commented Feb 19, 2017

andyferris commented Feb 19, 2017

andreasnoack commented Feb 19, 2017 •

edited

Loading

c42f commented Feb 20, 2017

andreasnoack commented Feb 20, 2017

timholy commented Feb 20, 2017

andreasnoack commented Feb 20, 2017

coveralls commented Feb 20, 2017

andyferris commented Feb 20, 2017

andreasnoack commented Feb 21, 2017

andyferris commented Feb 22, 2017

c42f commented Feb 22, 2017

andyferris Feb 22, 2017

c42f Feb 22, 2017

andyferris Feb 22, 2017

andyferris commented Feb 22, 2017

andyferris Feb 23, 2017

andreasnoack commented Feb 23, 2017

andyferris commented Feb 23, 2017

andyferris commented Feb 23, 2017

andreasnoack commented Feb 23, 2017 •

edited

Loading

coveralls commented Feb 23, 2017

andyferris commented Feb 23, 2017

andreasnoack commented Feb 23, 2017

c42f commented Feb 27, 2017

andyferris commented Feb 27, 2017

andreasnoack commented Feb 27, 2017

andyferris commented Feb 27, 2017

Use new inner constructor and type declaration syntax #107

Use new inner constructor and type declaration syntax #107

Conversation

andreasnoack commented Feb 17, 2017

andyferris commented Feb 19, 2017

andyferris commented Feb 19, 2017

andreasnoack commented Feb 19, 2017 • edited Loading

c42f commented Feb 20, 2017

andreasnoack commented Feb 20, 2017

timholy commented Feb 20, 2017

andreasnoack commented Feb 20, 2017

coveralls commented Feb 20, 2017

andyferris commented Feb 20, 2017

andreasnoack commented Feb 21, 2017

andyferris commented Feb 22, 2017

c42f commented Feb 22, 2017

andyferris Feb 22, 2017

Choose a reason for hiding this comment

c42f Feb 22, 2017

Choose a reason for hiding this comment

andyferris Feb 22, 2017

Choose a reason for hiding this comment

andyferris commented Feb 22, 2017

andyferris Feb 23, 2017

Choose a reason for hiding this comment

andreasnoack commented Feb 23, 2017

andyferris commented Feb 23, 2017

andyferris commented Feb 23, 2017

andreasnoack commented Feb 23, 2017 • edited Loading

coveralls commented Feb 23, 2017

andyferris commented Feb 23, 2017

andreasnoack commented Feb 23, 2017

c42f commented Feb 27, 2017

andyferris commented Feb 27, 2017

andreasnoack commented Feb 27, 2017

andyferris commented Feb 27, 2017

andreasnoack commented Feb 19, 2017 •

edited

Loading

andreasnoack commented Feb 23, 2017 •

edited

Loading