adriank / ObjectPath

The agile query language for semi-structured data
http://objectpath.org
MIT License
380 stars 93 forks source link

timedelta problems #66

Closed redhog closed 6 years ago

redhog commented 6 years ago

Microsecond handling seems to not be working properly

$ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0,0,1])"""))'
00:00:00.000001
$ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0]) - time([0,0,0,1])"""))'
23:59:59.009999

but

>>> datetime.datetime(1970,1,1,0,0) - datetime.datetime(1970,1,1,0,0,0,1)
datetime.timedelta(-1, 86399, 999999)

Notice the number of 9:s

Another sign of the strange microsecond handling is that subtraction of certain values fail:

$ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0,0,999999])"""))'
00:00:00.999999
$ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0]) - time([0,0,0,999999])"""))'
Traceback (most recent call last):
  File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/core/interpreter.py", line 158, in exe
    return fst-snd
TypeError: unsupported operand type(s) for -: 'datetime.time' and 'datetime.time'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/core/interpreter.py", line 646, in execute
    ret=exe(tree)
  File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/core/interpreter.py", line 164, in exe
    return timeutils.subTimes(fst,snd)
  File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/utils/timeutils.py", line 169, in subTimes
    return datetime.time(*reversed(t2))
ValueError: microsecond must be in 0..999999
adriank commented 6 years ago

Have you tried doing the same in Python directly? Date and time calculations are not trivial and require A LOT of work to support edge cases (literally millions of edge cases). Three first examples you pasted here are probably working properly. The last one requires support for datetime.time

Greetings, Adrian Kalbarczyk

http://kalbarczyk.co

On Thu, Aug 23, 2018 at 5:50 PM Egil Möller notifications@github.com wrote:

Microsecond handling seems to not be working properly

$ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0,0,1])"""))' 00:00:00.000001 $ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0]) - time([0,0,0,1])"""))' 23:59:59.009999

but

datetime.datetime(1970,1,1,0,0) - datetime.datetime(1970,1,1,0,0,0,1) datetime.timedelta(-1, 86399, 999999)

Notice the number of 9:s

Another sign of the strange microsecond handling is that subtraction of certain values fail:

$ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0,0,999999])"""))' 00:00:00.999999 $ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0]) - time([0,0,0,999999])"""))' Traceback (most recent call last): File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/core/interpreter.py", line 158, in exe return fst-snd TypeError: unsupported operand type(s) for -: 'datetime.time' and 'datetime.time'

During handling of the above exception, another exception occurred:

Traceback (most recent call last): File "", line 1, in File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/core/interpreter.py", line 646, in execute ret=exe(tree) File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/core/interpreter.py", line 164, in exe return timeutils.subTimes(fst,snd) File "/home/ubuntu/env/lib/python3.6/site-packages/objectpath/utils/timeutils.py", line 169, in subTimes return datetime.time(*reversed(t2)) ValueError: microsecond must be in 0..999999

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/adriank/ObjectPath/issues/66, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKycjzZ6YFvECJabmvCIuzr4bCD3J4Dks5uTs8vgaJpZM4WJzjC .

redhog commented 6 years ago

This is in python:

>>> datetime.datetime(1970,1,1,0,0) - datetime.datetime(1970,1,1,0,0,0,1)
datetime.timedelta(-1, 86399, 999999)
>>> (datetime.datetime(1970,1,1,0,0) - datetime.datetime(1970,1,1,0,0,0,1)).microseconds
999999

Here the result has 6 nines.

Four nines does not make any sense - if the last parameter is microseconds the result would have 6 nines, and if it was milliseconds it would have 3 nines.

/Egil

adriank commented 6 years ago

Now I understand what you meant. :)

Strange, here https://github.com/adriank/ObjectPath/blob/master/objectpath/utils/timeutils.py#L152 I explicitly set milliseconds to 10000, not to 1000000. Maybe change of Python specs across time? I don't know if changing 10000 to 1000000 would break other things so I leave it as is for now. If you have time to check if changing this line doesn't break anything, please do so.

redhog commented 6 years ago

Opened a pull request that fixes all of the above. It doesn't seem to break anything, just fixes both of the above problems.. I have tested the above code, plus made sure all the units tests run properly on the PR.

redhog commented 6 years ago

Oh, + has the same problem!

$ python -c 'import objectpath; print(objectpath.Tree({}).execute("""time([0,0,0,500000]) + time([0,0,0,500000])"""))'
00:00:01.990000

So, I updated the PR, fixing the addTimes problem too.