quinnj / Datetime.jl

A Date and DateTime implementation for Julia; Now deprecated in favor of https://github.com/quinnj/Dates.jl
Other
20 stars 15 forks source link

"Overflowing" datetime operator returns a value #59

Closed randyzwitch closed 10 years ago

randyzwitch commented 10 years ago

This is a fun behavior:


#Correct
julia> datetime(2013,7,1,12,0,0,0,offset(hours(-6)))
2013-07-01T12:00:00 -06:00

#Gigantic hours number
julia> datetime(2013,7,1,1259,0,0,0,offset(hours(-6)))
2013-08-22T11:00:00 -06:00

Feels like we might want to attach an error to hours > 23 like there is on the month:

#No 13th month
julia> datetime(2013,13,1,12,0,0,0,offset(hours(-6)))

BoundsError()
while loading In[28], in expression starting on line 1
 in totaldays at /home/rzwitch/.julia/v0.3/Datetime/src/Datetime.jl:101
 in datetime at /home/rzwitch/.julia/v0.3/Datetime/src/Datetime.jl:153 (repeats 2 times)
quinnj commented 10 years ago

Yeah, I've struggled to know what to do here. On the one hand, I'd hate putting so many "valid range" checks into the constructor and kill performance; on the other hand, I know it's pretty lame that the only period we don't "roll forward" with is months, though maybe we should. It could be as simple as adding a divrem(m,12) to get how many months to roll forward.

quinnj commented 10 years ago

For reference, I know Go just does rolling forward with everything, while Java 8 now checks every input. So there's not a general consensus, though we could at least be consistent.

randyzwitch commented 10 years ago

Does checking the inputs really cause that much of a performance hit? It feels like the behavior should be to always throw an error; I wouldn't want to silently get an incorrect timestamp because I passed the wrong argument in (which is how I goofed around into this behavior)

quinnj commented 10 years ago

Well, probably not much for Date, but more so for DateTime. It can also be considered a feature, because sometimes you have 30 hours to input and want it to roll forward, but I realize that's not a really strong argument. Basically, I'd love a solid argument one way or the other to finally get my butt off the fence.

quinnj commented 10 years ago

This is fixed in Base and the Dates.jl package where inputs are validated on Date/DateTime creation.

StefanKarpinski commented 10 years ago

How about this strategy: do the paranoid version that checks everything and see if anyone complains. If they do, add an unsafe method for constructing a DateTime without doing all the checks and let the normal DateTime constructor do the checks and then call that?

quinnj commented 10 years ago

That sounds like a good idea. I made the change a few weeks ago and no complaints yet, so we'll see.