Compat for at-vectorize_(1|2)arg deprecation

[yuyichao]

Provide Compat.@vectorize_1arg and Compat.@vectorize_2arg as an upgrade
path for packages.

[yuyichao]

We also need a better @compat for the broadcast syntax for this to be usable.

[tkelman]

This is backwards. Should try to provide the new syntax if possible, not just move the old macro here.

[yuyichao]

I know this is backward but that's the point.

The issue is that even though packages can use the 0.5 broadcast syntax with Compat (well, apart from the fact that the return type is wrong, which I'm trying to fix now) they can't remove the vectorized function without breaking all the users, which means that without the macro in this PR the package will either

1. Break user code
2. Can't fix depwarn
3. Roll their own @vectorize_* macro or write them out manually, only to be removed later once all users are upgraded.

As the commit message says, this is basically an easy upgrade path for packages that uses the macro so that they can fix their own use of the vectorized form and make sure user code won't break with minimum change of code.

[yuyichao]

The provide the new syntax if possible part is #196, which is not good enough for packages that uses @vectorize_* to use yet.

[yuyichao]

OK, the return type of the broadcast syntax is fixed in most cases. This makes it as good as comprehension on 0.3/0.4 although not as good as 0.5.

[yuyichao]

Haven't added test for the splatting case yet, will do that later.

[tkelman]

What's the upgrade path? Packages can just replace uses of the vectorize macros with calls to broadcast.

[yuyichao]

Not for packages that uses the macro to define public functions

[tkelman]

Is the problem just that the base deprecated version of the macro doesn't give deprecation warnings when you call the vectorized functions that it defines? We could change that. I don't think Compat should be a holding ground for deprecated code.

[yuyichao]

No the issue is that the macro gives a depwarn when used.

I don't think Compat should be a holding ground for deprecated code.

That's not new and the alternative is to move the same code to all other packages.

[tkelman]

As it should, it's deprecated. There have only been a handful of cases of putting the deprecated version here, and those were due to major differences in what would work on old julia versions - functorize and Compat.ASCIIString. The deprecated macro now giving deprecation warnings isn't a good reason to put it here without the warnings. There's an easy workaround here of defining vectorized methods manually.

[yuyichao]

There's an easy workaround here of defining vectorized methods manually.

That could be said for the macro before it was deprecate, as well as the aliases for string renaming. In fact, I was doing the string deprecation fix by defining the type in packages before all of them are available in Compat.

The defining vectorized method manually is also not that "easy". For one, the deprecation should only be raised on 0.6 (if not more precise than that) so there has to be a version check. The broadcast / shape / promotion code is also different between versions so it's non-trivial to manually defining these functions in packages in a way that works on all julia versions.

In general, I think this package should define common things that help people fixing depwarns. In most case that's by defining new functions on old julia versions. However, there are some cases when that's not possible and it should be ok to define necessary piece that helps in those cases.

[yuyichao]

Also, being a clear import/qualified use from Compat, it's a clear self documentation that something can be removed when support for old julia version is dropped

[tkelman]

I don't think Compat should host code that's obsolete from the very start. The way in which this package is intended to help people fix deprecation warnings is by supporting the new syntax on old Julia versions. A LegacyMacros package under JuliaArchive would be a better place for things that are deprecated in base that you still want packages to use without changes. People should stop using these macros, that's the point of deprecating them. If we put them here they'll never die.

[yuyichao]

obsolete from the very start.

I don't see why that's the case. I'm only saying that it's not easier to define those functions manually than what it was before it was deprecated. It was useful which is why people use it. And FWIW, whether the code is obsolete from the start has little to do with how the upgrade should be done, how many users exists does.

The way in which this package is intended to help people fix deprecation warnings is by supporting the new syntax on old Julia versions.

That's one way and happens to be what works most of the time. It's unfortunately not always the case.

A LegacyMacros package under JuliaArchive would be a better place for things that are deprecated in base that you still want packages to use without changes.

That could be a good place for the @ngenerate and related macros. This is not one of those since this does not make it possible to use the old code without depwarn. The macro still give a depwarn when the function defined is called and people should be aware of that and move away from actually using the function defined.

As mentioned before, the correct upgrade path for a package that uses these macros is,

1. Deprecate the use of the defined functions

   This is done automatically with both Compat.@vectorize_* and Base.@vectorize_*

2. Fix use of the defined functions

3. Mark that the function defined by the macro are deprecated and should be removed.

   This is achieve by the Compat., which should be a mark for something needs to be done, and possibly some comment where the macro is used.

4. When enough/all users are migrated, remove the old definitions (i.e. the use of the Compat. macros).

It is true that it can theoretically take one release cycle longer to be removed them from Compat. Whether that will actually happen depends on how many users of the macro have got to step 4 when we drop 0.5 support from Compat. Given that most (all?) users of the macro drops julia version supports much faster than Compat, I don't think that's a real issue. (In the worst case, we just need to raise a depwarn from the Compat version for one release cycle after 0.5 support is dropped from Compat).

[TotalVerb]

I wonder if it might be worth having an additional module Deprecations.jl (or perhaps Compat.Deprecations) that contains helper functions to deprecate functionality. Examples of exported macros may include Deprecations.@deprecate, Deprecations.@deprecate_vectorize_1arg, Deprecations.@deprecate_binding, etc. To me, this functionality does not seem like "compatibility" functionality, but rather convenient macros to generate methods that have depwarn calls included.

[yuyichao]

Well, it's "compatibility" in that it's to make the code being able to run on multiple julia versions.

I don't see why putting it in a module would help anything apart from making using Compat harder but if that makes people happy I can certainly do that.

[TotalVerb]

Putting it in a module keeps related items together, and I think this @deprecate_vectorize_(1|2)arg is definitely more similar to @deprecate or @deprecate_binding (which the new module can import from Base, and re-export) than it is to the existing compatibility code. It also opens the way for more complex deprecation patterns to be added, such as the currently-non-existent @deprecate_without_replacement, or @deprecate_with_message, which cannot be included in Compat because they make little sense here.

[yuyichao]

Also it's not always deprecated, only on 0.6 (to avoid breaking users, ref JuliaInterop/ZMQ.jl#109), so calling it deprecate will be strange.

[TotalVerb]

Hmm right, that is an argument for keeping it in Compat, since it has different behaviour across versions.

[tkelman]

Very few users or package developers should be caring about deprecation warnings on 0.6-dev yet. By the time they do, a much simpler upgrade path is just not calling the base vectorize macro on 0.6-dev or newer.

[yuyichao]

What's the "Base vectorize macro"?

[tkelman]

put the call to vectorize_1arg behind a version conditional in the packages that use it.

[yuyichao]

How is that different from manually defining the function in packages which I've already argued against above?

[yuyichao]

Also, Very few users or package developers should be caring about deprecation warnings on 0.6-dev yet. is not a very good argument to be made if the argument of base bisectability on packages needs to be made at other places.

[tkelman]

Deprecation warnings don't usually make much difference to bisectability. Migrating package code to the new syntax is the right way to get rid of the deprecations, and going through putting this code here is a distraction from accomplishing that.

[yuyichao]

Return type of the broadcasting syntax with scalar input is fixed. Also add more tests.

[yuyichao]

Why should Compat continue providing old code after it goes through a base deprecation cycle?

Because the base change is actually two sequential deprecations. Of course they are closely related which is why this might not need to stay for longer than base. Another potential reason is that Compat support more julia versions (3 now) than Base (only 1).

Implementing the equivalent of master's dep_vectorize seems better.

Good. As I said, this is exactly what this PR does.

[tkelman]

Good. As I said, this is exactly what this PR does.

But with a different name from what master uses, which is confusing.

[tkelman]

In other words I think it should be called dep_vectorize_1arg here, and the functions it defines should also be deprecations on earlier julia versions if we really want people to move away from them.

[yuyichao]

The @dep_vectorize_* is a internal name that users shouldn't actually care about which is why I picks the name that should be more clear what it is doing on earlier julia versions.

Now if I use dep_.... here, what should it do on 0.3-0.5? and (less importantly) should it be exported?

[yuyichao]

the functions it defines should also be deprecations on earlier julia versions if we really want people to move away from them.

It'll be much longer than 30 lines but sure.

[TotalVerb]

It should deprecate the vectorized versions on 0.3 to 0.5 also.

[yuyichao]

It'll be much longer than 30 lines but sure.

Actually maybe not assuming we want the right depwarn and can accept minor behavior difference.

[tkelman]

these aren't exported from base so I don't think they should be exported here

[yuyichao]

Sure. Do note that the version here and the version in base serves completely different purposes. The base version isn't a public API, this one is.

[tkelman]

We could make the base one more official, in which case packages could choose to call that on 0.6 instead of the one that throws a deprecation warning.

[yuyichao]

Well, a compatibility layer in base that doesn't work on multiple julia version isn't very useful.
Packages will have to do version check in order to use the macro in which case they might as well use the Compat version or break API on 0.6.
This will basically means that the @dep_* macro needs to be kept in Base for one version longer than the @vectorize_ which I think is a worse choice, at least for compatibility layer, than putting them here.

[yuyichao]

The performance of the broadcast syntax should almost match that of comprehension on 0.4 and the same syntax on master now. The main reason it doesn't is due to broadcast_shape being much slower than master. Since that is a Base issue that's fixed, I don't feel like backporting a better broadcast_shape implementation to 0.3/0.4. It's not a regression compare to current Compat master and can always be done later if anyone needs it.

[yuyichao]

Will merge later today.

Note that this is backportable to 0.8.x if people want it on 0.3.

[stevengj]

Shouldn't they raise a depwarn when 0.4 support is dropped? dot-call syntax is implemented in 0.5, so once people are using 0.5+ there is no need for @vectorize.

[yuyichao]

This is just the most conservative option since hypothetical a package that is deprecated on 0.6 but works on 0.5 don't need to remove the use of @dep_vectorize_ since @vectorize_ is not deprecated there. Hopefully packages will migrate fast enough and we don't need to worry about this too much....