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

Deprecate cat(dim, Xs...) to cat(Xs..., dims=dim) #27163

Merged
merged 1 commit into from
May 22, 2018
Merged

Conversation

mbauman
Copy link
Sponsor Member

@mbauman mbauman commented May 18, 2018

Fixes #27100. I have also started on a bigger refactor here that will more easily allow for custom arrays to extend concatenations, but that ended up being not as simple as I had hoped, so this PR simply moves to the dims keyword argument and hides all internal dispatch so Base only exposes one cat method without restrictions on its arguments for now.

@mbauman mbauman added the deprecation This change introduces or involves a deprecation label May 18, 2018
@mbauman mbauman added this to the 1.0 milestone May 18, 2018
@mbauman
Copy link
Sponsor Member Author

mbauman commented May 21, 2018

Man, rough go on CI: FreeBSD timed out, both Travis Linuxes got stuck/segfaulted waiting for a git username/password, and CircleCI isn't even loading for me, so I cannot see what was going on in the 32bit build there.

@KristofferC
Copy link
Sponsor Member

32bit CI loads for me and got:

 Got exception ErrorException("Process timed out possibly waiting for a response. Process output found:\n\"\"\"\n\r\nsignal (15): Terminated\r\nin expression starting at no file:0\r\n_ZN4llvm13ConstantRange21makeAllowedICmpRegionENS_7CmpInst9PredicateERKS0_ at /tmp/julia/bin/../lib/julia/libLLVM-6.0.so (unknown line)\r\n_ZN4llvm13ConstantRange24makeSatisfyingICmpRegionENS_7CmpInst9PredicateERKS0_ at /tmp/julia/bin/../lib/julia/libLLVM-6.0.so (unknown line)\r\n_ZN4llvm13ConstantRange19makeExactICmpRegionENS_7CmpInst9PredicateERKNS_5APIntE at /tmp/julia/bin/../lib/julia/libLLVM-6.0.so (unknown line)\r\n\n\"\"\"") outside of a @test
  Process timed out possibly waiting for a response. Process output found:
  """
  

  signal (15): Terminated

  in expression starting at no file:0

  _ZN4llvm13ConstantRange21makeAllowedICmpRegionENS_7CmpInst9PredicateERKS0_ at /tmp/julia/bin/../lib/julia/libLLVM-6.0.so (unknown line)

  _ZN4llvm13ConstantRange24makeSatisfyingICmpRegionENS_7CmpInst9PredicateERKS0_ at /tmp/julia/bin/../lib/julia/libLLVM-6.0.so (unknown line)

  _ZN4llvm13ConstantRange19makeExactICmpRegionENS_7CmpInst9PredicateERKNS_5APIntE at /tmp/julia/bin/../lib/julia/libLLVM-6.0.so (unknown line)

  
  """
  Stacktrace:
   [1] error(::String, ::String) at ./error.jl:42
   [2] (::getfield(Main.Test36Main_LibGit2_libgit2.LibGit2Tests, Symbol("##5#8")){Int32,Cmd,Array{Any,1},getfield(Main.Test36Main_LibGit2_libgit2.LibGit2Tests, Symbol("#format_output#7")){Bool},Base.GenericIOBuffer{Array{UInt8,1}}})(::RawFD, ::Base.TTY) at /tmp/julia/share/julia/stdlib/v0.7/LibGit2/test/libgit2.jl:93
   [3] with_fake_pty(::getfield(Main.Test36Main_LibGit2_libgit2.LibGit2Tests, Symbol("##5#8")){Int32,Cmd,Array{Any,1},getfield(Main.Test36Main_LibGit2_libgit2.LibGit2Tests, Symbol("#format_output#7")){Bool},Base.GenericIOBuffer{Array{UInt8,1}}}) at /tmp/julia/share/julia/test/TestHelpers.jl:37
...

@mbauman mbauman changed the title Deprecate cat(dim, Xs...) to cat(Xs..., dim=dim) Deprecate cat(dim, Xs...) to cat(Xs..., dims=dim) May 21, 2018
@Keno
Copy link
Member

Keno commented May 21, 2018

I think we may have to increase those timeouts for libgit2

@JeffBezanson
Copy link
Sponsor Member

Note on the merge conflict: I moved this code to util.jl (with the rest of the text-color-related code, which I did to resolve a bootstrap order thing, but it makes sense anyway).

@@ -86,8 +86,8 @@ Arrays can be constructed and also concatenated using the following functions:
| Function | Description |
|:---------------------- |:---------------------------------------------------- |
| [`cat(k, A...)`](@ref) | concatenate input n-d arrays along the dimension `k` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cat(k, A...)](@ref) ...

it should be cat(A...; dims = k), isn't it?

@JeffBezanson JeffBezanson merged commit a916cbc into master May 22, 2018
@JeffBezanson JeffBezanson deleted the mb/depcatdim branch May 22, 2018 18:41
Liozou pushed a commit to Liozou/julia that referenced this pull request May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants