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.

Sign up for GitHub

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

Jump to bottom

Deprecate adding integers to CartesianIndex #26284

Merged
merged 1 commit into from
Mar 2, 2018
Merged

Deprecate adding integers to CartesianIndex #26284

merged 1 commit into from
Mar 2, 2018

Conversation

mbauman
Copy link
Sponsor Member

@mbauman mbauman commented Mar 1, 2018

fixes #26227

@mbauman mbauman added domain:arrays [a, r, r, a, y, s] kind:deprecation This change introduces or involves a deprecation labels Mar 1, 2018
@mbauman mbauman force-pushed the mb/deprecateaddcartesianindextoint branch from 8d89fd5 to e721972 Compare March 1, 2018 21:40
@mbauman
Copy link
Sponsor Member Author

mbauman commented Mar 1, 2018

I should note that I checked the native code between the old definition and the deprecation... and they're identical (modulo line numbers).

@JeffBezanson JeffBezanson merged commit 8fff818 into master Mar 2, 2018
@JeffBezanson JeffBezanson deleted the mb/deprecateaddcartesianindextoint branch March 2, 2018 19:24
mbauman added a commit that referenced this pull request Mar 5, 2018
@mbauman
Merge remote-tracking branch 'origin/master' into mb/deprecatemultiva…
95a78df 
…luenonscalarindexedassignment

* origin/master: (28 commits)
  fix an optimizer bug in `invoke` with non-constant functions (#26301)
  lower top-level statements in such a way that the front-end knows (#26304)
  Make sure Sockets page has h1 header (#26315)
  fix doctests, and make them less prone to errors (#26275)
  FIx intro to manual chapter on types (#26312)
  Add a missing "that" (#26313)
  fix docstring for code_llvm (#26266)
  Remove the examples/ folder (#26153)
  download cert.pem from deps-getall, fixes #24903 (#25344)
  Slight update to doc string for at-enum to refer to instances (#26208)
  performance tweak in reverse(::String) (#26300)
  remove references to `TCPSocket` in Base docstrings (#26298)
  Deprecate adding integers to CartesianIndex (#26284)
  Deprecate conj(::Any), add real(::Missing) and imag(::Missing) (#26288)
  fix #26267, regression in `names` with `imported=false` (#26293)
  fix #25857, `typeinfo` problem in showing arrays with abstract tuple types (#26289)
  Add adjoint(::Missing) method (#25502)
  Use lowered form in logging macro (#26291)
  deprecate bin, oct, dec, hex, and base in favor of `string` and keyword args (#25804)
  deprecate `spawn(cmd)` to `run(cmd, wait=false)` (#26130)
  ...
@timholy timholy mentioned this pull request Sep 30, 2018
Closed
@timholy
Copy link
Sponsor Member

timholy commented Sep 30, 2018

Because one(x) is supposed to return a multiplicative identity for x (see the docstring), this is a little problematic:

julia> x = CartesianIndex(2,3)
CartesianIndex(2, 3)

julia> one(x)*x
ERROR: MethodError: no method matching *(::CartesianIndex{2}, ::CartesianIndex{2})
Closest candidates are:
  *(::Any, ::Any, ::Any, ::Any...) at operators.jl:502
  *(::Integer, ::CartesianIndex{N}) where N at multidimensional.jl:115
  *(::CartesianIndex, ::Integer) at multidimensional.jl:116
Stacktrace:
 [1] top-level scope at none:0

oneunit seems more appropriate:

julia> CartesianIndex{3}(1)
CartesianIndex(1, 1, 1)

@timholy timholy mentioned this pull request Sep 30, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JeffBezanson JeffBezanson JeffBezanson approved these changes

Assignees
No one assigned
Labels
domain:arrays [a, r, r, a, y, s] kind:deprecation This change introduces or involves a deprecation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deprecate +(::CartesianIndex, ::Number)
3 participants
@mbauman @timholy @JeffBezanson