Conversation
stdlib/Dates/src/types.jl
Outdated
| microsecond(a) == microsecond(b) && nanosecond(a) == nanosecond(b) | ||
| end | ||
| (==)(x::TimeType, y::TimeType) = (===)(promote(x, y)...) | ||
| Base.hash(x::Time, h::UInt) = hash(hour(x), hash(second(x), hash(second(x), h))) |
There was a problem hiding this comment.
Did you mean to include minute instead of second twice?
There was a problem hiding this comment.
Yeah, I was having source build problems, so I didn't get a chance to try any of this out locally. I think I've got it sorted out now.
|
These commits will have to be separated. The hash change is a bug fix (whether it's suitable for a point release is debatable, but it can definitely go in 1.1), but the deprecation will take longer. For now I'd recommend moving the deprecated methods to deprecated.jl as you've done, but with no warnings for now, until we decide how to handle them. |
|
Ok, I commented out the |
|
|
||
| # 1.0 deprecations | ||
| function (+)(x::AbstractArray{<:TimeType}, y::GeneralPeriod) | ||
| # depwarn("non-broadcasted arithmetic is deprecated for Dates.TimeType; use broadcasting instead", nothing) |
There was a problem hiding this comment.
All the nothings should be updated too?
There was a problem hiding this comment.
to what? I admit I don't really know what should be there instead of nothing.
There was a problem hiding this comment.
the name of the function that's being deprecated (in this case :(+))
also mark all functions that contain a call to depwarn with @noinline so we can print the warning with correct location info
|
This seems fine for now. The details of the deprecations can be settled when we decide to active them. |
|
Is the first commit backportable? |
|
I think so; this fixes the hash function to obey the correctness property, so it's definitely a bug fix. |
|
Ok, it seems the first commit is already included in 1.0.2: #29742. |
No description provided.