Add is_ok and is_err typeguard functions

[mishamsk]

I'd say this is a more performant implementation than #72 as it doesn't use is instance checks at all.

Added tests, changelog & took a stab at main readme (although this is up to maintainers, whether this should be advertised as a better approach).

On my laptop one of the mypy tests is failing on 3.9, but that's not related to my change (complains about | where Union should be)

closes #69

[francium]

Just a few minor things.

Otherwise PR looks great. Thank you for the help.

@wbolster if you have any thought or opinions on this, since you were working in this area prior, feel free to take a look and share your comments.

[francium]

Thanks again @mishamsk

Few minor typos that I'll take care of post merge.

[francium]

Note to self, small typo here, remove the . at the end of safe

[francium]

Note to self, small typo here, remove the extra - at the start of this line

[francium]

Note to self, perhaps make methods and functions bold to distinguish them since is_ok and is_err have the same names

[wbolster]

sorry for late reply. #72 suffered from some type checker limitations, and this one does not. do i understand that correctly?

I'd say this is a more performant implementation than #72 as it doesn't use isinstance() checks at all.

i think this is misguided. why would isinstance() checks be slow? in cpython, these usually boil down to a direct pointer comparison, which is always much faster than a method call on a custom class.

[mishamsk]

sorry for late reply. #72 suffered from some type checker limitations, and this one does not. do i understand that correctly?

unfortunately not, it is all the same. But a new PEP is being drafted which will "fix" that

I'd say this is a more performant implementation than #72 as it doesn't use isinstance() checks at all.

i think this is misguided. why would isinstance() checks be slow? in cpython, these usually boil down to a direct pointer comparison, which is always much faster than a method call on a custom class.

Oh, I assumed it should be slower, but apparently I am wrong. Although it may be different in 3.11 with reduced "price" for additional frames and specializing attribute getters, which I'd hope would speed up bound methods getters as well... Thanks for the tip about isinstance checks. Do you happen to have a good link handy that explores how CPython implements them by any chance?

[wbolster]

i think this is misguided. why would isinstance() checks be slow? in cpython, these usually boil down to a direct pointer comparison, which is always much faster than a method call on a custom class.

Thanks for the tip about isinstance checks. Do you happen to have a good link handy that explores how CPython implements them by any chance?

@mishamsk

this should help:

• isinstance() (python) corresponds to PyObject_IsInstance (c api)
• PyObject_IsInstance (https://github.com/python/cpython/blob/v3.11.4/Objects/abstract.c#L2682C1-L2682C20) delegates to object_recursive_isinstance()
• object_recursive_isinstance() (https://github.com/python/cpython/blob/v3.11.4/Objects/abstract.c#L2617C1-L2617C28) has a fast path using Py_IS_TYPE (https://github.com/python/cpython/blob/v3.11.4/Objects/abstract.c#L2620C9-L2620C19)
• Py_IS_TYPE is a macro (docs) expanding to Py_TYPE(o) == type
• Py_TYPE (docs) boils down to return ob->ob_type (source)

… so the simple cases boil down to a bit of pointer chasing and a pointer comparison, which is all very fast.

[mishamsk]

@wbolster thanks a lot! I got curious, and run a very dumb test (sorry for formatting, doing it from iPad while on vacation):

import timeit

class T:
    cattr = True

    def __init__(self) -> None:
        self.attr = True

    @property
    def prop(self) -> bool:
        return True

    def ist(self) -> bool:
        return True
    
class T1:
    __slots__ = ("attr",)

    cattr = True

    def __init__(self) -> None:
        self.attr = True

    @property
    def prop(self) -> bool:
        return True

    def ist(self) -> bool:
        return True

t = T()

inst_check = timeit.timeit("isinstance(t, T)", globals=globals())
fn_check = timeit.timeit("t.ist()", globals=globals())
prop_check = timeit.timeit("t.prop", globals=globals())
attr_check = timeit.timeit("t.attr", globals=globals())
cattr_check = timeit.timeit("t.cattr", globals=globals())

print(f"Non-slotted. Isintance: {inst_check}. Method: {fn_check}. Prop: {prop_check}. Inst attr: {attr_check}. Class attr: {cattr_check}")

t = T1()

inst_check = timeit.timeit("isinstance(t, T)", globals=globals())
fn_check = timeit.timeit("t.ist()", globals=globals())
prop_check = timeit.timeit("t.prop", globals=globals())
attr_check = timeit.timeit("t.attr", globals=globals())
cattr_check = timeit.timeit("t.cattr", globals=globals())

print(f"Slotted. Isintance: {inst_check}. Method: {fn_check}. Prop: {prop_check}. Inst attr: {attr_check}. Class attr: {cattr_check}")

Which gives for 3.10 & 3.11 (in docker running on synology NAS;-), but we do not care about absolute numbers):

py310: commands[0]> python isinstance_test.py
Non-slotted. Isintance: 0.16529686748981476. Method: 0.22358105331659317. Prop: 0.20174939185380936. Inst attr: 0.10315892100334167. Class attr: 0.10350674390792847
Slotted. Isintance: 0.2414640188217163. Method: 0.1983325481414795. Prop: 0.20569878816604614. Inst attr: 0.09698593616485596. Class attr: 0.08725540339946747
py310: OK ✔ in 1.99 seconds
py311: commands[0]> python isinstance_test.py
Non-slotted. Isintance: 0.0721818059682846. Method: 0.10906121879816055. Prop: 0.16895851492881775. Inst attr: 0.03669581562280655. Class attr: 0.0829509049654007
Slotted. Isintance: 0.12867401540279388. Method: 0.10731867700815201. Prop: 0.17078260332345963. Inst attr: 0.035740092396736145. Class attr: 0.06810076534748077
  py310: OK (1.99=setup[0.30]+cmd[1.68] seconds)
  py311: OK (1.05=setup[0.01]+cmd[1.04] seconds)
  congratulations :) (3.19 seconds)

So, apparently, speedwise the optimal way would be to create an instance attribute (something like __is_ok__) and use it, which will be quicker than isinstance checks. But, until the PEP I’ve mentioned is approved, is_ok/is_err functions would still be more limiting than isinstance checks typing wise.

PS. surprised to see that property access is slower than a method call on py311. I guess some of their call optimisations do not work for descriptors🤷‍♂️