tests fail in 2038

[bmwiedemann]

BUG/PROBLEM REPORT / FEATURE REQUEST

What I did:

osc co openSUSE:Factory/python-DateTime && cd $_
osc build --vm-type=kvm --noservice --clean --build-opt=--vm-custom-opt="-rtc base=2039-09-02T06:07:00" --alternative-project=home:bmwiedemann:reproducible openSUSE_Tumbleweed

What I expect to happen:

tests should keep working in the future (at least 16 years)

What actually happened:

similar to issue #41 in that there is a rounding error in DateTime-5.2 that can randomly break a test.

 =================================== FAILURES ===================================
 __________________________ DateTimeTests.test_pickle ___________________________
 
 self = <DateTime.tests.test_datetime.DateTimeTests testMethod=test_pickle>
 
     def test_pickle(self):
         dt = DateTime()
         data = pickle.dumps(dt, 1)
         new = pickle.loads(data)
         for key in DateTime.__slots__:
 >           self.assertEqual(getattr(dt, key), getattr(new, key))
 E           AssertionError: 2198556426235027 != 2198556426235026
 
 src/DateTime/tests/test_datetime.py:253: AssertionError
 =============================== warnings summary ===============================

What version of Python and Zope/Addons I am using:

openSUSE-Tumbleweed-20230729 python3.10

[bmwiedemann]

still there in DateTime-5.4

[icemac]

@d-maurer Do have an idea what's going wrong here?

[d-maurer]

Michael Howitz wrote at 2024-3-19 00:24 -0700:

@d-maurer Do have an idea what's going wrong here?

`DateTime` uses a `float` in its state representation (representing seconds since the epoch). A `float` has limited precision. This limited precision is the cause of the error as demontrated by the following transscript: ```pyconsole

>> from DateTime import DateTime >> f=DateTime() >> f.__setstate__((2198556426.235027, False, 'GMT+1')) >> f

DateTime('2039/09/02 07:07:6.235027 GMT+1')

>> f.__getstate__()

(2198556426.235026, False, 'GMT+1') ``` As we see, the `__getstate__` result has lost the last bit. If we want to get rid of errors like this, we must avoid a `float` in the `__getstate__` representation. One possibility would be to replace the `float` by a pair "(integral) seconds since epoch", "remaining microseconds". Another one to replace the `float` by "microseconds since the epoch". to use

[perrinjerome]

For reference:

DateTime/src/DateTime/DateTime.py

Lines 448 to 468 in e892a9d

	def __getstate__(self):
	# We store a float of _micros, instead of the _micros long, as we most
	# often don't have any sub-second resolution and can save those bytes
	return (self._micros / 1000000.0,
	getattr(self, '_timezone_naive', False),
	self._tz)
	def __setstate__(self, value):
	if isinstance(value, tuple):
	self._parse_args(value[0], value[2])
	self._micros = long(value[0] * 1000000)
	self._timezone_naive = value[1]
	else:
	for k, v in value.items():
	if k in self.__slots__:
	setattr(self, k, v)
	# BBB: support for very old DateTime pickles
	if '_micros' not in value:
	self._micros = long(value['_t'] * 1000000)
	if '_timezone_naive' not in value:
	self._timezone_naive = False

this was set as a float to have shorter pickles, this was apparently wrong for this reason. In our case ( cc @fdiary ), we considered that it is wrong because of the precision loss and we are using a monkey patch to export the int in __getstate__ ( for this and other historical reasons ).

Unlike with protocol 1, a float is no longer serialized shorter pickle than an int with pickle protocol 3:

>>> pickletools.dis(pickle.dumps(2198556426.235026, 3))
    0: \x80 PROTO      3
    2: G    BINFLOAT   2198556426.235026
   11: .    STOP
highest protocol among opcodes = 2
>>> pickletools.dis(pickle.dumps(int(2198556426.235026 * 1000000.0), 3))
    0: \x80 PROTO      3
    2: \x8a LONG1      2198556426235026
   11: .    STOP
highest protocol among opcodes = 2

for the fix, I think we can consider exporting the int "as is" during __getstate__ and add more conditions during the __setstate__ to support the new format.

[d-maurer]

Jérome Perrin wrote at 2024-3-19 04:36 -0700:

... for the fix, I think we can consider exporting the int "as is" during `__getstate__` and add more conditions during the `__setstate__` to support the new format.

I assume this `int` represents "microseconds since epoch`. In this case, would you like to prepare a PR (as you apparently have already most of the code for it).

[perrinjerome]

I assume this int represents "microseconds since epoch`.

exactly:

DateTime/src/DateTime/DateTime.py

Lines 861 to 865 in 868de71

	# self._micros is the time since the epoch
	# in long integer microseconds.
	if microsecs is None:
	microsecs = long(round(t * 1000000.0))
	self._micros = microsecs

I submitted #62 with this change we discussed.

I tried to run osc myself but I had errors when trying to create an account, @bmwiedemann can you try easily from the linked PR ?

[bmwiedemann]

looks good in a quick test.

[icemac]

Fix released in https://pypi.org/project/DateTime/5.5/