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

DateIndex._convert(): special calculation of timestamp value #123

Open
drfho opened this issue Jul 8, 2021 · 3 comments
Open

DateIndex._convert(): special calculation of timestamp value #123

drfho opened this issue Jul 8, 2021 · 3 comments

Comments

@drfho
Copy link
Contributor

drfho commented Jul 8, 2021

Hello,
the timestamp calculation in
https://github.com/zopefoundation/Products.ZCatalog/blob/master/src/Products/PluginIndexes/DateIndex/DateIndex.py#L141
looks somehow surprising: is there a special reason for e.g. "yr*12"?
In my opion a unix timestamp may be more adequate for the job, because its easier to process the timestamps back into a valid datetime value.
Or is there any special reason to use that "internal" timestamp format and not a standard unix value for the zcatalog-index?

LINE 141
-         t_val = ((((yr * 12 + mo) * 31 + dy) * 24 + hr) * 60 + mn)
+        t_struct = datetime(yr,mo,dy,hr,mn).timetuple()
+        t_val = time.mktime(t_struct)

Any hint is welcome,
looking forward
fh

@andbag
Copy link
Member

andbag commented Jul 9, 2021

I don't know why the standard libraries were not used. Maybe there was a 2037 issue in previous python or unix (32bit) versions. At least the time unit of t_val is minutes and the type of the variable is not signed integer. I.e. your code should be like this

t_val = int(time.mktime(t_struct)/60)

The value is also missing the minutes that have passed until 1970. Once the code should be changed, the result of the new method is not the same of the old method. Maybe tests must be rewritten and all datetime indexes of running systems with datetime indexes must be rebuilt.

@drfho
Copy link
Contributor Author

drfho commented Jul 9, 2021

Sorry, the old code ((((yr * 12 + mo) * 31 + dy) * 24 + hr) * 60 + mn) seems not to compute minutes. I have no idea what the resulting number might be Any idea?.

Py code snippet for testing

import time
from datetime import date
from datetime import datetime
from DateTime.DateTime import DateTime

yr = 2021
mo = 6
dy = 9
hr = 12
mn = 0

### OLD
t_val = ((((yr * 12 + mo) * 31 + dy) * 24 + hr) * 60 + mn)
print("OLD", int(t_val))

### NEW
t_struct = datetime(yr,mo,dy,hr,mn).timetuple()
t_val = time.mktime(t_struct)
print("NEW",int(t_val))
>> OLD 1082890800
>> NEW 1623232800

Check Timestaps: https://www.confirado.de/tools/timestamp-umrechner.html

@d-maurer
Copy link
Contributor

d-maurer commented Jul 9, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants