slashmili / python-jalali

Jalali calendar binding for Python based on Python's datetime module
http://pypi.python.org/pypi/jdatetime/
Other
335 stars 46 forks source link

fix(date, datetime): return NotImplemented for unknown type operations #139

Closed 5j9 closed 10 months ago

5j9 commented 1 year ago

This is how the built-in date and datetime work.

closes #84

hramezani commented 1 year ago

Thanks @5j9 for the patch. It's a breaking change for python jalali and we need a major release. what do you think @slashmili ? please add a changelog in https://github.com/slashmili/python-jalali/blob/main/CHANGELOG.md

slashmili commented 1 year ago

True, requires a major version. I think(might be wrong) this error format was how the datetime in python2 used to work

5j9 commented 1 year ago

Just to be clear, although this is a breaking change, but the behaviour for almost all usual cases has not changed at all. datetime.date.today() + 1 still raises TypeError: unsupported operand type(s) for +: 'datetime.date' and 'int'.

NotImplemented is just a special value that tells the interpreter to retry the operation on the other operand and if that fails too, then the interpreter will raise TypeError as before.

The breaking behaviour may occur if:

  1. the right operand is implemented in a way that can handle jdatetime.date class but they rely on date to raise a TypeError and prevent the operation. An extremely unlikely scenario.
  2. someone is calling date.__add__(object) instead of date + object and relying on the TypeError raised. More plausible.
hramezani commented 12 months ago

@slashmili I think we can merge it. what do you think?

slashmili commented 11 months ago

@hramezani Yes!

I'd say we only bump the minor version and hope for the best, what do you think?

otherwise we have to keep this change until next major release which we don't have any plan to do so

hramezani commented 11 months ago

@hramezani Yes!

I'd say we only bump the minor version and hope for the best, what do you think?

otherwise we have to keep this change until next major release which we don't have any plan to do so

Sounds good to me 👍

slashmili commented 10 months ago

Thanks @5j9 🎉