gweis / isodate

ISO 8601 date/time parser
BSD 3-Clause "New" or "Revised" License
155 stars 59 forks source link

Python 3.8 compatibility #58

Closed JoseKilo closed 2 years ago

JoseKilo commented 5 years ago

Using python3.8, with -Werror

>>> import datetime
>>> import isodate
>>> datetime.datetime.now() + isodate.parse_duration("P5Y")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "[...]/.tox/py38-std-sha256/lib/python3.8/site-packages/isodate/duration.py", line 183, in __add__
    newdt = other.replace(year=newyear, month=newmonth, day=newday)
DeprecationWarning: an integer is required (got type decimal.Decimal).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.

It looks like casting to int explicitly would solve the warning, something like:

ewdt = other.replace(year=int(newyear), month=int(newmonth), day=newday)

According to the release notes: https://docs.python.org/3.8/whatsnew/3.8.html

Many builtin and extension functions that take integer arguments will now emit a deprecation warning for Decimals, Fractions and any other objects that can be converted to integers only with a loss (e.g. that have the __int__() method but do not have the __index__() method). In future version they will be errors. (Contributed by Serhiy Storchaka in bpo-36048.)

hugovk commented 3 years ago

Fixed in the fork at https://github.com/isodate/isodate/pull/1, also ditches Travis CI for GitHub Actions.

ftfishg commented 2 years ago

@hugovk Thanks for the fix!

In your commit you also converted newday to int whereas the original code seems to allow floats for days. The check above seems to imply the day can be floats.

            if (not(float(self.years).is_integer() and
                    float(self.months).is_integer())):
                raise ValueError('fractional years or months not supported'
                                 ' for date calculations')

Also, maybe we should add a check for other here too. We mostly only add two values together, if we use int() in the end, one of them (e.g. self.months) is known to be integral, the other (other.month) has to be integral too in order to for the conversion to be lossless.

Could you please have another look? Thanks again!

hugovk commented 2 years ago

Perhaps it would help to open a new issue and include example code showing how this worked before and what the expected output would be before compared to now?