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

Add compatibility for cat with dims kw argument #613

Merged
merged 3 commits into from
Aug 22, 2018

Conversation

galenlynch
Copy link
Contributor

cat(dims, Xs...) was deprecated to cat(Xs..., dims=dim) in #27163, but as
described in #612 support for the new signature is missing in Compat.

Fixes #612

cat(dims, Xs...) was deprecated to cat(Xs...; dims=dim) in [#27163], but as
described in JuliaLang#612 support for the new signature is missing in Compat.

Fixes JuliaLang#612

[#27163]: https://github.com/JuliaLang/julia/issue/27163
Copy link
Member

@martinholters martinholters left a comment

Choose a reason for hiding this comment

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

LGTM modulo the README change and maybe the test change.

README.md Outdated
@@ -294,6 +294,8 @@ Currently, the `@compat` macro supports the following syntaxes:

* `squeeze` with `dims` as keyword argument ([#26660]).

* `cat` with `dims` as keyword argument ([#27163])
Copy link
Member

Choose a reason for hiding this comment

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

Should be Compat.cat to make clear that one needs to access it with explicit qualification.

@test Compat.cat([1, 2], [3, 4, 5], dims = 1) == [1, 2, 3, 4, 5]
@test Compat.cat([1, 2], [3, 4], dims = 2) == [1 3; 2 4]
if VERSION < v"0.7.0-DEV.5165"
@test_throws UndefKeywordError Compat.cat([1, 2], [3, 4])
Copy link
Member

Choose a reason for hiding this comment

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

This throws again after removal of the respective deprecation for 1.0 (should be v"1.0.0-DEV.57"), so we could (should?) test there, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, then we're effectively testing the implementation in base. At the time that I wrote it, It at least seemed weird to me. I'll defer to you and other members of Compat, but I don't see a reason to test code outside of Compat.

Copy link
Member

@martinholters martinholters Aug 21, 2018

Choose a reason for hiding this comment

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

Of, course, most (all?) our tests effectively test the implementation in Base on recent Julia. But here it is admittedly a little different, as we would do so more explicitly. I have no strong feelings about this. Maybe someone else wants to chime in? Otherwise I'll merge this as is tomorrow.

@martinholters martinholters merged commit 5b939bf into JuliaLang:master Aug 22, 2018
martinholters added a commit that referenced this pull request Oct 8, 2019
martinholters added a commit that referenced this pull request Oct 8, 2019
martinholters added a commit that referenced this pull request Oct 8, 2019
martinholters added a commit that referenced this pull request Oct 8, 2019
martinholters added a commit that referenced this pull request Oct 8, 2019
martinholters added a commit that referenced this pull request Oct 13, 2019
* Bump required Julia version to 1.0

* Remove compatibility support code for:
  * `at-__MODULE__` (from #363)
  * `devnull`, `stdin`, `stdout`, and `stderr` from #499
  * `at-nospecialize` (from #385 and #409)
  * `isabstracttype` and `isconcretetype` (from #477)
  * `invokelatest` from #424
  * array-like access to `Cmd` from #379
  * `Val(n)` and `ntuple`/`reshape` with `Val` from #381 and #399
  * `logdet(::Any)` fallback from #382
  * `chol(::UniformScaling)` from #382
  * `pushfirst!`, `popfirst!` from #444
  * `fieldcount` from #386
  * `read(obj, ::Type{String})` from #385 and #580
  * `InexactError`, `DomainError`, and `OverflowError` constructors from #393
  * `corrected` kw arg to `cov` from #401
  * `adjoint` from #401
  * `partialsort` from #401
  * `pairs` from #428
  * `AbstractRange` from #400
  * `rtoldefault` from #401
  * `Dates.Period` rounding from #462
  * `IterativeEigensolvers` from #435
  * `occursin` from #520
  * `Char` concatenation from #406
  * `BitSet` from #407
  * `diagm` and `spdiagm` with pairs from #408
  * `Array` c'tors from `UniformScaling` from #412 and #438
  * `IOContext` ctor taking pairs from #427
  * `undef` from #417 and #514
  * `get` on `ENV` from #430
  * `ComplexF...` from #431
  * `tr` from #614
  * `textwidth` from #644
  * `isnumeric` from #543
  * `AbstractDict` from #435
  * `axes` #435 and #442
  * `Nothing` and `Cvoid` from #435
  * `Compat.SuiteSparse` from #435
  * `invpermute!` from #445
  * `replace` with a pair from #445
  * `copyto!` from #448
  * `contains` from #452
  * `CartesianIndices` and `LinearIndices` from #446, #455, and #524
  * `findall` from #466 (and #467).
  * `argmin` and `argmax` from #470, #472, and #622
  * `parentmodule` from #461
  * `codeunits` from #474
  * `nameof` from #471
  * `GC` from #477
  * `AbstractDisplay` from #482
  * `bytesavailable` from #483
  * `firstindex` and `lastindex` from #480 and #494
  * `printstyled` from #481
  * `hasmethod` from #486
  * `objectid` from #486
  * `Compat.find*` from #484 and #513
  * `repr` and `showable` from #497
  * `Compat.names` from #493 and #505
  * `Compat.round` and friends #500, #530, and #537
  * `IOBuffer` from #501 and #504
  * `range` with kw args and `LinRange` from #511
  * `cp` and `mv` from #512
  * `indexin` from #515
  * `isuppercase` and friends from #516
  * `dims` and `init` kwargs from #518, #528, #590, #592, and #613
  * `selectdim` from #522 and #531
  * `repeat` from #625
  * `fetch(::Task)` from #549
  * `isletter` from #542
  * `isbitstype` from #560
  * `at-cfunction` from #553 and #566
  * `codeunit` and `thisind` and friends from #573
  * `something` from #562
  * `permutedims` from #582
  * `atan` from #574
  * `split` and `rsplit` from #572
  * `mapslices` from #588
  * `floatmin` and `floatmax` from #607
  * `dropdims` from #618
  * required keyword arguments from #586
  * `CartesianRange` in `at-compat` from #377
  * `finalizer` from #416
  * `readline`, `eachline`, and `readuntil` from #477, #541, and #575
  * curried `isequal`, `==`, and `in` from #517
  * `Some` from #435 and #563
  * `at-warn` and friends from #458

* Remove old deprecations

* Deprecate:
  * `Compat.Sockets` from #545 and #594
  * `TypeUtils` from #304
  * `macros_have_sourceloc` from #355
  * `Compat.Sys` from #380, #433, and #552
  * `Compat.MathConstants` from #401
  * `Compat.Test`, `Compat.SharedArrays`, `Compat.Mmap`, and `Compat.DelimitedFiles` from #404
  * `Compat.Dates` from #413
  * `Compat.Libdl` from #465 (and #467)
  * `AbstractDateTime` from #443
  * `Compat.Printf` from #435
  * `Compat.LinearAlgebra` from #463
  * `Compat.SparseArrays` from #459
  * `Compat.Random` from #460, #601, and #647
  * `Compat.Markdown` from #492
  * `Compat.REPL` from #469
  * `Compat.Serialization` from #473
  * `Compat.Statistics` from #583
  * `Fix2` from #517
  * `Compat.Base64` from #418
  * `Compat.Unicode` from #432 and #507
  * `notnothing` from #435 and #563
  * `Compat.IteratorSize` and `Compat.IteratorEltype` from #451
  * `enable_debug(::Bool)` from #458
  * `Compat.Distributed` from #477
  * `Compat.Pkg` from #485
  * `Compat.InteractiveUtils` from #485
  * `Compat.LibGit2` from #487
  * `Compat.UUIDs` from #490
  * `Compat.qr` from #534
  * `Compat.rmul!` from #546
  * `Compat.norm` abd friends from #577

* Remove obsolete README entry, missed in #385

* Remove obsolete tests (e.g. missed in #372)

* Remove obsolete `VERSION` conditionals and some minor clean-up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants