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

oneunit of unitfull matrix should return a unitfull matrix #30227

Merged
merged 7 commits into from
Dec 3, 2018
Merged

oneunit of unitfull matrix should return a unitfull matrix #30227

merged 7 commits into from
Dec 3, 2018

Conversation

KlausC
Copy link
Contributor

@KlausC KlausC commented Dec 1, 2018

Applying oneunit to an matrix of unitfull element type should return a matrix of same type.

julia> using Dates
julia> oneunit([Second(0) Second(0); Second(0) Second(1)] )
2×2 Array{Second,2}:
 1 second   0 seconds
 0 seconds  1 second 

but we have currently:

julia> oneunit([Second(0) Second(0); Second(0) Second(1)] )
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Second
Closest candidates are:
  convert(::Type{Second}, ::Minute) at /home/julia/julia/usr/share/julia/stdlib/v1.1/Dates/src/periods.jl:407
  convert(::Type{Second}, ::Hour) at /home/julia/julia/usr/share/julia/stdlib/v1.1/Dates/src/periods.jl:407
  convert(::Type{Second}, ::Day) at /home/julia/julia/usr/share/julia/stdlib/v1.1/Dates/src/periods.jl:407
  ...
Stacktrace:
 [1] setindex! at ./array.jl:779 [inlined]
 [2] _one(::Second, ::Array{Second,2}) at ./array.jl:487
 [3] oneunit(::Array{Second,2}) at ./array.jl:493
 [4] top-level scope at none:0

@KlausC KlausC changed the title Krc/oneunitarray oneunit of a unitfull array should return a unitfull array Dec 1, 2018
@KlausC KlausC changed the title oneunit of a unitfull array should return a unitfull array oneunit of a unitfull matrix should return a unitfull array Dec 1, 2018
@KlausC KlausC changed the title oneunit of a unitfull matrix should return a unitfull array oneunit of unitfull matrix should return a unitfull matrix Dec 1, 2018
@stevengj
Copy link
Member

stevengj commented Dec 2, 2018

Looks like a bug, thanks!

By the way, why does this file still have a docstring for ones? Wasn’t that deprecated?

@StefanKarpinski
Copy link
Member

By the way, why does this file still have a docstring for ones? Wasn’t that deprecated?

I thought so too, but apparently it's still there and exported, along with zeros:

julia> ones
ones (generic function with 6 methods)

help?> ones
search: ones leading_ones trailing_ones countlines count_ones to_indices RoundNearest

  ones([T=Float64,] dims...)

  Create an Array, with element type T, of all ones with size specified by dims. See
  also: fill, zeros.

  Examples
  ≡≡≡≡≡≡≡≡≡≡

  julia> ones(1,2)
  1×2 Array{Float64,2}:
   1.0  1.0

  julia> ones(ComplexF64, 2, 3)
  2×3 Array{Complex{Float64},2}:
   1.0+0.0im  1.0+0.0im  1.0+0.0im
   1.0+0.0im  1.0+0.0im  1.0+0.0im

julia> zeros
zeros (generic function with 6 methods)

help?> zeros
search: zeros count_zeros set_zero_subnormals get_zero_subnormals leading_zeros

  zeros([T=Float64,] dims...)

  Create an Array, with element type T, of all zeros with size specified by dims. See
  also fill, ones.

  Examples
  ≡≡≡≡≡≡≡≡≡≡

  julia> zeros(1)
  1-element Array{Float64,1}:
   0.0

  julia> zeros(Int8, 2, 3)
  2×3 Array{Int8,2}:
   0  0  0
   0  0  0

@Sacha0, didn't you delete these?

@StefanKarpinski StefanKarpinski merged commit 15ce5ef into JuliaLang:master Dec 3, 2018
@StefanKarpinski StefanKarpinski added bugfix This change fixes an existing bug minor change Marginal behavior change acceptable for a minor release labels Dec 3, 2018
@StefanKarpinski StefanKarpinski added this to the 1.1 milestone Dec 3, 2018
@fredrikekre
Copy link
Member

We tried: #25507

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Dec 3, 2018

Oh wow, I totally misremembered that as actually happening. I guess it was the removal of all the uses that I remembered. Eh, I'm fairly ok with still having ones and zeros, doesn't really hurt.

@KristofferC KristofferC added the needs news A NEWS entry is required for this change label Dec 3, 2018
@Sacha0
Copy link
Member

Sacha0 commented Dec 4, 2018

@Sacha0, didn't you delete these?

@fredrikekre made a valiant attempt, but alas... :)

We tried: #25507

❤️

@KlausC KlausC deleted the krc/oneunitarray branch December 4, 2018 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug minor change Marginal behavior change acceptable for a minor release needs news A NEWS entry is required for this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants