Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in /base/ #16498

pkofod · 2016-05-21T12:55:13Z

Not quite done, but almost. All tests (besides some string-stuff) pass locally on ubuntu.

yuyichao · 2016-05-21T13:22:01Z

base/array.jl


-cell(dims::Integer...)   = Array(Any, dims...)
-cell(dims::Tuple{Vararg{Integer}}) = Array(Any, convert(Tuple{Vararg{Int}}, dims))
+cell(dims::Integer...)   = Array{Any}( dims...)


Here's an extra space before dims

yuyichao · 2016-05-21T13:22:41Z

What are the string failure? Shouldn't the two constructors be equivalent?

pkofod · 2016-05-21T14:50:29Z

Sorry, I really explained myself poorly. There was a string test error before changing anything and it was there after as well. It seems to have disappeared when rebasing.

Edit: I will run the tests again locally just to be sure. It doesn't seem to appear on the CIs.

mbauman · 2016-05-21T15:03:45Z

base/docs/helpdb/Base.jl

@@ -7376,7 +7376,7 @@ for the effects of compilation.
    Array(dims)

 `Array{T}(dims)` constructs an uninitialized dense array with element type `T`. `dims` may
-be a tuple or a series of integer arguments. The syntax `Array(T, dims)` is also available,
+be a tuple or a series of integer arguments. The syntax `Array{T}(dims)` is also available,


You don't want to change this one. It's interesting that it's documented as deprecated, but I guess we never actually made it show a deprecation warning.

The next step will be to actually deprecate these:

julia/base/boot.jl

Lines 315 to 320 in 99bf23d

# TODO: possibly turn these into deprecations

Array{T,N}(::Type{T}, d::NTuple{N,Int}) = Array{T}(d)

Array{T}(::Type{T}, d::Int...) = Array{T}(d)

Array{T}(::Type{T}, m::Int) = Array{T,1}(m)

Array{T}(::Type{T}, m::Int,n::Int) = Array{T,2}(m,n)

Array{T}(::Type{T}, m::Int,n::Int,o::Int) = Array{T,3}(m,n,o)

. Running the tests with them deprecated will be an extra sanity check that you got them all.

Yes of course, that is a mistake.

I will add the deprecation.

Okay, I tried, and I have to say, I'm not sure how to do the deprecation. I understand it should be something like

@deprecate old new

but if I try something like

@deprecate Array{T,N}(::Type{T}, d::NTuple{N,Int}) = Array{T}(d)

I get something like (plus a lot more backtracing)

deprecated.jl require.jl error during bootstrap: LoadError(at "sysimg.jl" line 337: LoadError(at "deprecated.jl" line 1186: Base.MethodError(f=Base.#@deprecate(), args=(Expr(:call, Expr(:curly, :Array, :T, :N)::Any, Expr(:::, Expr(:curly, :Type, :T)::Any)::Any, Expr(:::, :d, Expr(:curly, :NTuple, :N, :Int)::Any)::Any)::Any = Expr(:block, Expr(:line, 1186, :deprecated.jl)::Any, Expr(:call, Expr(:curly, :Array, :T)::Any, :d)::Any)::Any,)))) rec_backtrace at /home/pkm/julia5/julia/src/stackwalk.c:83 record_backtrace at /home/pkm/julia5/julia/src/task.c:228 jl_throw at /home/pkm/julia5/julia/src/task.c:514 eval at /home/pkm/julia5/julia/src/interpreter.c:415 jl_parse_eval_all at /home/pkm/julia5/julia/src/ast.c:773 jl_load at /home/pkm/julia5/julia/src/toplevel.c:583 #218 at ./sysimg.jl:188 jl_call_method_internal at /home/pkm/julia5/julia/src/julia_internal.h:78 do_call at /home/pkm/julia5/julia/src/interpreter.c:58 eval at /home/pkm/julia5/julia/src/interpreter.c:174 jl_toplevel_eval_flex at /home/pkm/julia5/julia/src/toplevel.c:545 jl_toplevel_eval_flex at /home/pkm/julia5/julia/src/toplevel.c:433 jl_toplevel_eval_flex at /home/pkm/julia5/julia/src/toplevel.c:443 jl_parse_eval_all at /home/pkm/julia5/julia/src/ast.c:777 jl_load at /home/pkm/julia5/julia/src/toplevel.c:583 exec_program at /home/pkm/julia5/julia/ui/repl.c:483 true_main at /home/pkm/julia5/julia/ui/repl.c:550 main at /home/pkm/julia5/julia/ui/repl.c:671 unknown function (ip: 0x7f7298be0a3f) unknown function (ip: 0x401788)

or if I delete all the methods you linked to

docs/core.jl inference.jl A method error occurred before the base module was defined. Aborting... Array (Any, 9) while loading inference.jl, in expression starting on line 3467 rec_backtrace at /home/pkm/julia5/julia/src/stackwalk.c:83 jl_method_error_bare at /home/pkm/julia5/julia/src/gf.c:965 jl_method_error at /home/pkm/julia5/julia/src/gf.c:976 jl_apply_generic at /home/pkm/julia5/julia/src/gf.c:1581 Type at ./inference.jl:92 jl_call_method_internal at /home/pkm/julia5/julia/src/julia_internal.h:78 typeinf_edge at ./inference.jl:1498 jl_call_method_internal at /home/pkm/julia5/julia/src/julia_internal.h:78 typeinf at ./inference.jl:1528 jl_call_method_internal at /home/pkm/julia5/julia/src/julia_internal.h:78 macro expansion; at ./inference.jl:3468 jl_call_method_internal at /home/pkm/julia5/julia/src/julia_internal.h:78 jl_parse_eval_all at /home/pkm/julia5/julia/src/ast.c:777 jl_load at /home/pkm/julia5/julia/src/toplevel.c:583 include at ./boot.jl:222 jl_call_method_internal at /home/pkm/julia5/julia/src/julia_internal.h:78 do_call at /home/pkm/julia5/julia/src/interpreter.c:58 eval at /home/pkm/julia5/julia/src/interpreter.c:174 jl_toplevel_eval_flex at /home/pkm/julia5/julia/src/toplevel.c:545 jl_toplevel_eval_flex at /home/pkm/julia5/julia/src/toplevel.c:433 jl_toplevel_eval_flex at /home/pkm/julia5/julia/src/toplevel.c:443 jl_toplevel_eval_in_warn at /home/pkm/julia5/julia/src/builtins.c:547 eval at ./boot.jl:225 jl_call_method_internal at /home/pkm/julia5/julia/src/julia_internal.h:78 do_call at /home/pkm/julia5/julia/src/interpreter.c:58 eval at /home/pkm/julia5/julia/src/interpreter.c:174 jl_toplevel_eval_flex at /home/pkm/julia5/julia/src/toplevel.c:545 jl_parse_eval_all at /home/pkm/julia5/julia/src/ast.c:777 jl_load at /home/pkm/julia5/julia/src/toplevel.c:583 exec_program at /home/pkm/julia5/julia/ui/repl.c:483 true_main at /home/pkm/julia5/julia/ui/repl.c:550 main at /home/pkm/julia5/julia/ui/repl.c:671 unknown function (ip: 0x7f81f1155a3f) unknown function (ip: 0x401788) Allocations: 298087 (Pool: 298048; Big: 39); GC: 0

I know I'm doing something wrong, I just can't figure out what.

@deprecate Array{T,N}(::Type{T}, d::NTuple{N,Int}) Array{T}(d) no =.

(The first error message says you are calling @deprecate with the wrong number of arguments), the second error message says you haven't fix the inference.jl for the deprecated constructor yet.

@deprecate Array{T,N}(::Type{T}, d::NTuple{N,Int}) Array{T}(d) no =.

Obviously, thanks. And thanks, I'll have a look at inference.jl.

(Please quote macros.... Apparently there are many creative usernames on github.....)

(Please quote macros.... Apparently there are many creative usernames on github.....)

(Sorry, didn't notice the quote was stripped when quoting using the "r" shortcut )

tkelman · 2016-05-22T07:07:33Z

This is great. I think we could merge it as is, and have a followup PR to add the deprecation and clean up any last stragglers?

pkofod · 2016-05-22T08:36:25Z

Fine by me. I have not been outside of base/, so test/ needs to be cleaned up as well.

Edit: I am working on test/ locally, but if this is merged before I finish, I'll just make a new PR.
Edit^2: I am done, but I'm running tests locally before pushing, so I don't waste a CI run if I missed a bracket :)

pkofod · 2016-05-22T19:12:53Z

Some further changes. I still don't fully understand how I should do the deprecation. I can't really find any information about the content and structure of Inference.jl, and consequently I'm unsure what I need to change.

I take it the commits will simply be squashed upon merge, so I don't need to do that myself, yes?

nalimilan · 2016-05-22T21:23:12Z

I'm not sure why you want to modify inference.jl, but have a look at deprecated.jl.

JeffBezanson · 2016-05-22T21:28:58Z

Are we really sure we want to deprecate Array(T, dims)? Is there a discussion of this somebody can point me to?

I guess we will want the array constructor to be Array{T}(iterable[, size]), which will make it the same as Set, Dict, etc.

I also certainly hope inference.jl doesn't need to be modified for this.

yuyichao · 2016-05-22T21:29:41Z

As for the issue you see in type inference. It seems to be caused by type comprehension. T[i for i in a] is lowered to a call to Array(T, n) atm.

pkofod · 2016-05-23T07:41:04Z

Are we really sure we want to deprecate Array(T, dims)? Is there a discussion of this somebody can point me to?

Are you referring to the changes in this PR currently, or if Array(T, dims) should still be allowed? I kinda of just noticed the deprecation note in the documentation when I was doing another PR, and figured I would clean the code according to the doc text. On the question of actually deprecating Array(T, dims) I guess this could be simply left out of this PR. It seems to be that you introduced the "deprecated" comment yourself
23a6995 so I guess there hasn't necessarily been any discussion whether to actually deprecate it or not since then.

I'm not sure why you want to modify inference.jl, but have a look at deprecated.jl.

and

I also certainly hope inference.jl doesn't need to be modified for this.

Well, if I deprecate the functions mbauman mentioned in comments from a previous commit, I get an error in inference.jl when makeing, which is apparantly due to

As for the issue you see in type inference. It seems to be caused by type comprehension. T[i for i in a] is lowered to a call to Array(T, n) atm.

Regarding CI, it looks like a timeout at AppVeyor x86_64.

tkelman · 2016-05-24T01:44:59Z

doc/manual/metaprogramming.rst

@@ -102,7 +102,7 @@ objects:
    julia> dump(ex2)
    Expr
      head: Symbol call
-      args: Array(Any,(3,))
+      args: Array{Any}(3,))


this is missing one (

tkelman · 2016-05-24T01:49:45Z

Maybe we don't have to deprecate the old way, but I think this is a bit better style and more generalizable to other array types without them all having to implement the "eltype as first positional argument" pattern. Besides, I think that pattern is somewhat flawed - ref #11557.

tkelman · 2016-05-24T08:17:08Z

doc/manual/constructors.rst

@@ -586,7 +586,7 @@ to another, you should probably define a ``convert`` method instead.
 On the other hand, if your constructor does not represent a lossless
 conversion, or doesn't represent "conversion" at all, it is better
 to leave it as a constructor rather than a ``convert`` method.  For
-example, the ``Array(Int)`` constructor creates a zero-dimensional
+example, the ``Array{Int}`` constructor creates a zero-dimensional


this one is specifically documenting the paren method

Thanks for pointing it out. I will go through everything and look for these mistakes. I'm wondering, though. Should it be Array(Int) or Array{Int}() (supporting two syntaxes for one thing... :) ) ? I've changed it to the latter in the actual code.

StefanKarpinski · 2016-05-24T14:36:57Z

I'm strongly in favor of deprecating the old way. Having too many ways to write things is just confusing.

tkelman · 2016-05-24T21:03:40Z

Let's hold off on making this pr any bigger and just merge it when green then, unless anyone sees any other cases that are confusing or typos. The deprecation can be a followup.

pkofod · 2016-05-25T08:07:49Z

Git error on AppVeyor.
edit oh, and merge conflicts :)

… arrays.

…base/pkg/.

…base/linalg/.

…base/strings/.

…base/unicode/.

…base/grisu/.

…base/fft/.

…base/ for files starting b-e.

…base/docs/.

…base/ for files starting with f-i.

…base/dSFMT.jl, base/Enums.jl, and base/LineEdit.jl.

…base/ for files starting with l-m.

…base/ for files starting with p-r.

…base/ for files starting with s-t.

…base/special/.

pkofod · 2016-05-25T08:50:16Z

Not exactly sure what is happening with AppVeyor. If someone feels like it, you can stop the current runs on both CIs, and I can try to fire them again. I rebased twice within a short amount of time (I have a feeling that many commits are going to cause merge conflicts here... :) )

pkofod · 2016-05-25T09:00:17Z

Thanks. I'm not sure if you're doing this already @tkelman, but is it possible to stop the already running and queued runs ? ( https://travis-ci.org/JuliaLang/julia/pull_requests , https://ci.appveyor.com/project/JuliaLang/julia/history )

tkelman · 2016-05-25T09:15:07Z

There's some fail-fast code that automatically cancels them quickly when there are newer builds pending for the same PR. edit: that doesn't help for the ones that have already started though, so yeah those are worth cancelling manually

tkelman · 2016-05-25T11:38:19Z

doc/manual/interfaces.rst

 ============================================================================ ============================================ =======================================================================================
-


sphinx might not like deleting this line but we can deal with that later if it causes any issues

yuyichao reviewed May 21, 2016
View reviewed changes

mbauman reviewed May 21, 2016
View reviewed changes

pkofod force-pushed the array branch 3 times, most recently from 8b59080 to d5911d7 Compare May 21, 2016 21:25

pkofod changed the title ~~[WIP] Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in /base/~~ Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in /base/ May 22, 2016

tkelman reviewed May 24, 2016
View reviewed changes

pkofod force-pushed the array branch from f522c42 to fce29f8 Compare May 24, 2016 08:13

tkelman reviewed May 24, 2016
View reviewed changes

pkofod force-pushed the array branch from fce29f8 to e53c068 Compare May 24, 2016 08:17

pkofod force-pushed the array branch from 071b052 to 37fa7f4 Compare May 25, 2016 08:18

pkofod added 6 commits May 25, 2016 10:41

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) for…

effa667

… arrays.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

9ad7b84

…base/pkg/.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

c36e3ed

…base/linalg/.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

c25f198

…base/strings/.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

b3e3a3f

…base/unicode/.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

b5bca76

…base/grisu/.

pkofod added 12 commits May 25, 2016 10:41

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

1986cc4

…base/fft/.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

fd5c99c

…base/ for files starting b-e.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

a689764

…base/docs/.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

218a2eb

…base/ for files starting with f-i.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

a8fae4a

…base/dSFMT.jl, base/Enums.jl, and base/LineEdit.jl.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

0c31bfd

…base/ for files starting with l-m.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

922fdbf

…base/ for files starting with p-r.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

a7599e3

…base/ for files starting with s-t.

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in …

106f483

…base/special/.

Additional changes from Array(T, dim) to Array{T}(dim).

68d6eb8

Further changes.

9866e93

fix constructors.rst

b9057cf

pkofod force-pushed the array branch from 37fa7f4 to b9057cf Compare May 25, 2016 08:44

tkelman closed this May 25, 2016

tkelman reopened this May 25, 2016

tkelman reviewed May 25, 2016
View reviewed changes

tkelman merged commit 2152bf9 into JuliaLang:master May 25, 2016

tkelman mentioned this pull request May 25, 2016

Minor cleanup on BitArray constructors to fully allow for Integer types #16582

Merged

pabloferz mentioned this pull request Jun 8, 2016

misc. benchmark regressions since 0.4 #16128

Closed

tkelman mentioned this pull request Jul 23, 2016

RFC: Change Array(T, dims) to Array{T}(dims) #17583

Merged

KristofferC mentioned this pull request Oct 14, 2016

RFC: revert back to one index for some elementwise operations #18929

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in /base/ #16498

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in /base/ #16498

pkofod commented May 21, 2016

yuyichao May 21, 2016

yuyichao commented May 21, 2016

pkofod commented May 21, 2016 •

edited

Loading

mbauman May 21, 2016

pkofod May 21, 2016

pkofod May 21, 2016

yuyichao May 21, 2016

yuyichao May 21, 2016

pkofod May 21, 2016 •

edited by yuyichao

Loading

yuyichao May 21, 2016

pkofod May 21, 2016

tkelman commented May 22, 2016 •

edited

Loading

pkofod commented May 22, 2016 •

edited

Loading

pkofod commented May 22, 2016

nalimilan commented May 22, 2016

JeffBezanson commented May 22, 2016

yuyichao commented May 22, 2016

pkofod commented May 23, 2016

tkelman May 24, 2016

tkelman commented May 24, 2016 •

edited

Loading

tkelman May 24, 2016

pkofod May 24, 2016 •

edited

Loading

StefanKarpinski commented May 24, 2016

tkelman commented May 24, 2016

pkofod commented May 25, 2016 •

edited

Loading

pkofod commented May 25, 2016

pkofod commented May 25, 2016

tkelman commented May 25, 2016 •

edited

Loading

tkelman May 25, 2016

	# TODO: possibly turn these into deprecations
	Array{T,N}(::Type{T}, d::NTuple{N,Int}) = Array{T}(d)
	Array{T}(::Type{T}, d::Int...) = Array{T}(d)
	Array{T}(::Type{T}, m::Int) = Array{T,1}(m)
	Array{T}(::Type{T}, m::Int,n::Int) = Array{T,2}(m,n)
	Array{T}(::Type{T}, m::Int,n::Int,o::Int) = Array{T,3}(m,n,o)

		============================================================================ ============================================ =======================================================================================

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in /base/ #16498

Replace deprecated Array(T..., dims...) with Array{T...}(dims...) in /base/ #16498

Conversation

pkofod commented May 21, 2016

Choose a reason for hiding this comment

yuyichao commented May 21, 2016

pkofod commented May 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkofod May 21, 2016 • edited by yuyichao Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman commented May 22, 2016 • edited Loading

pkofod commented May 22, 2016 • edited Loading

pkofod commented May 22, 2016

nalimilan commented May 22, 2016

JeffBezanson commented May 22, 2016

yuyichao commented May 22, 2016

pkofod commented May 23, 2016

Choose a reason for hiding this comment

tkelman commented May 24, 2016 • edited Loading

Choose a reason for hiding this comment

pkofod May 24, 2016 • edited Loading

Choose a reason for hiding this comment

StefanKarpinski commented May 24, 2016

tkelman commented May 24, 2016

pkofod commented May 25, 2016 • edited Loading

pkofod commented May 25, 2016

pkofod commented May 25, 2016

tkelman commented May 25, 2016 • edited Loading

Choose a reason for hiding this comment

pkofod commented May 21, 2016 •

edited

Loading

pkofod May 21, 2016 •

edited by yuyichao

Loading

tkelman commented May 22, 2016 •

edited

Loading

pkofod commented May 22, 2016 •

edited

Loading

tkelman commented May 24, 2016 •

edited

Loading

pkofod May 24, 2016 •

edited

Loading

pkofod commented May 25, 2016 •

edited

Loading

tkelman commented May 25, 2016 •

edited

Loading