JuliaData / NamedTuples.jl

[DEPRECATED] NamedTuples.jl
Other
30 stars 17 forks source link

Updates to be compatible with 0.7 #52

Closed morris25 closed 5 years ago

morris25 commented 6 years ago

This PR deprecates any functions not supported by Base.NamedTuple in v0.7. It deprecates using @NT to declare a type so that: @NT(a, b) -> Deprecated in favor of NamedTuple{(:a, :b)} @NT(a::Int64, b::Float64) -> Deprecated in favor of NamedTuple{(:a, :b), Tuple{Int64, Float64}} @NT(a, b)(1, "hello") -> Deprecated in favor of NamedTuple{(:a, :b)}(1, "hello") @NT(a::Int64)(2.0) -> Deprecated in favor of NamedTuple{(:a,), Tuple{Int64}}(2.0)

@NT(a = 1, b = "hello") -> remains the same @NT(::Int64, ::Float64) -> Deprecated in favor of new macro@nt(::Int64, ::Float64)

It also adds Appveyor. Closes #51

omus commented 6 years ago

The deprecations on 0.7 should also be addressed: https://travis-ci.org/JuliaData/NamedTuples.jl/jobs/369297897

davidanthoff commented 6 years ago

Just a warning/heads up: I will upper bound all the packages from the Queryverse to not use this version on julia 0.6. That is a lot of packages, and some of them quite low-level, so in essence that would probably mean that a lot of folks would never get this update on 0.6.

My migration plan for Queryverse right now is to just do a breaking release once 0.7 is out that only works on 0.7, i.e. drop 0.6 support at that time. I do not plan to have a version that supports both 0.6 and 0.7 at the same time.

omus commented 6 years ago

My migration plan for Queryverse right now is to just do a breaking release once 0.7 is out that only works on 0.7, i.e. drop 0.6 support at that time. I do not plan to have a version that supports both 0.6 and 0.7 at the same time.

Completely reasonable plan. We still should try to support an easy transition for package that do want to support 0.6 and 0.7.

davidanthoff commented 6 years ago

I guess the question is whether that can be done without deprecations? The moment we introduce deprecations here, I'll have to add the upper bounds in my packages, and that will then hold things back for a lot of folks...

omus commented 6 years ago

I'm not sure what the issue with deprecations is. The deprecations which are being proposed here are to use a syntax which is much closer to what is being used in Julia 0.7 named tuples. If this gets merged there will be deprecations introduced for those using the NamedTuples package and the @NT macro for creating types. You would just need to update the deprecations in your own package like any other deprecation.

Overall this should help make your package work much closer to how it will work when you drop Julia 0.6 support.

omus commented 6 years ago

One potential gotcha with this PR we may want to address in the README is that on Julia 0.7:

julia> t = (a=1,)
(a = 1,)

julia> typeof(t) == NamedTuple{(:a,), Tuple{Int64}}
true

However with NamedTuples on Julia 0.6:

julia> t = NamedTuple{(:a,), Tuple{Int64}}(1)
(a = 1,)

julia> typeof(t) == NamedTuple{(:a,), Tuple{Int64}}
false

julia> typeof(t) <: NamedTuple{(:a,), Tuple{Int64}}
true

julia> typeof(t)
NamedTuple{(:a,), Tuple{Int64}}

The new show method definitely adds to the confusion.

davidanthoff commented 6 years ago

I'm not sure what the issue with deprecations is.

I just creates extra work: instead of just having one update cycle once julia 0.7 comes out, I would have to update packages twice, now to fix the deprecations, and then again when julia 0.7 comes around. At least for me the solution would be to just not fix the deprecations on 0.6 and instead add upper bounds in REQUIRE. From my point of view that works, but because some of the packages I would upper bound are so deep in the stack, it would mean that a lot of systems would never see this new release here, which might kind of defeat the purpose (not sure).

omus commented 6 years ago

@davidanthoff it seems odd to me that you're willing to put in the work to change the REQUIRE file to add an upper bound to a requirement but not revise the code to clean up deprecations.

I'll definitely say wait and see until this PR has stabilized before making any choices as to upper bounding or updating. If you do think it's too much work to support the next release I can (or one of my team members) do the update on your behalf.

omus commented 6 years ago

After looking over this code it may be best if we divide some of the functionality like this:

If we want to go 100% deprecation free we could leave @NT as is. The disadvantage of doing that is that it is less clear whether the macro will be returning a type or an instance.

davidanthoff commented 6 years ago

it seems odd to me that you're willing to put in the work to change the REQUIRE file to add an upper bound to a requirement but not revise the code to clean up deprecations.

Well, I think the etiquette in METADATA is that if you want to tag a new version that breaks/negatively affects other packages, you have to put the upper bounds into METADATA, so that should be no work for me :)

This is really just about my time budget: I just don't have the time right now to do two sweeps of deprecation updates, once for this and then again for 0.7. Introducing breaking changes that low in the stack is just hugely expensive for me. My guess is that there are about 20 packages that might need updating/testing, everything needs to be synchronized, tested in sync etc. Some of those packages I control, others I don't. A lot of that probably can't be outsourced to someone else, all the tagging/testing/coordinating would probably end up on my plate at the end of the day.

I guess my preference in general is that packages that have many dependencies and are low in the stack are super, super conservative in terms of deprecations/breaking changes. If they do make breaking changes, it just causes this enormous ripple effect of extra work for other folks. Sometimes that can't be avoided, but it would be great if it could be rare and we would try to avoid when at all possible.

I guess another option might be to just fork NamedTuples.jl: create a NamedTuples2.jl that works on julia 0.6 and 0.7, and folks that want to support both platforms could use that. The benefit of that would be that other packages could just stay on NamedTuples.jl for julia 0.6 and wouldn't require any extra work right now.

omus commented 6 years ago

In an effort to keep the changes to support Julia 0.7 to a minimum the critical parts of this PR have been made into #55.