connor-makowski / type_enforced

A pure python type enforcer for annotations. Enforce types in python functions and methods.
MIT License
45 stars 2 forks source link

False positive when returning child classes instances #25

Closed Ashark closed 10 months ago

Ashark commented 11 months ago

Hello. Thank you for the project. There is a problem when you try to return an object of a child class.

Let us say I have the following project structure.

MyPackage/__init__.py - empty file

MyPackage/Dev.py:

class Developer:
    def __init__(self):
        pass

MyPackage/PyDev.py:

from .Dev import Developer

class PythonDeveloper(Developer):
    def __init__(self):
        super().__init__()
        pass

main.py:

from MyPackage.Dev import Developer
from MyPackage.PyDev import PythonDeveloper
import type_enforced

@type_enforced.Enforcer
def my_func(way) -> Developer:  # meant that Developer instance or any Developer's child class instance is returned
    if way == 1:
        return Developer()
    elif way == 2:
        return PythonDeveloper()

print(my_func(1))  # <- all ok
print(my_func(2))  # <- Unexpectedly get exception
pass

The exception message is:

TypeError: (my_func): Type mismatch for typed variable `return`. Expected one of the following `[<class 'MyPackage.Dev.Developer'>]` but got `<class 'MyPackage.PyDev.PythonDeveloper'>` instead.

Why it is expecting only the base class instance? Can this be fixed?

connor-makowski commented 11 months ago

This is an interesting thought. Let me see what I can dig up here.

connor-makowski commented 10 months ago

I was able to get a fix working by checking the instance MRO items instead of the type itself. Another option is to take the passed type and work up the subclasses instead.

With that said, tests indicate that this will double type enforcement validation times. This will require some extra thought on my end.

As a fix for you now, you can add a simple helper method to work up the subclasses iteratively.

import type_enforced

def get_subclasses(obj):
    out = [obj]
    for i in obj.__subclasses__():
        out += get_subclasses(i)
    return out

class Developer:
    def __init__(self):
        pass

class PythonDeveloper(Developer):
    def __init__(self):
        super().__init__()
        pass

class SuperPythonDeveloper(PythonDeveloper):
    def __init__(self):
        super().__init__()
        pass

@type_enforced.Enforcer
def my_func(way) -> get_subclasses(Developer):  # meant that Developer instance or any Developer's child class instance is returned
    if way == 1:
        return Developer()
    elif way == 2:
        return PythonDeveloper()

print(my_func(1))  # <- all ok
print(my_func(2))  # <- No exception any more
Ashark commented 10 months ago

Thanks, I will use this workaround for now.

connor-makowski commented 10 months ago

After more thought, it seems less likely that type_enforced should automatically check subclasses.

A good example is bool types, which are also int types. In this case if you say a variable has to be a bool, but you actually pass an an int like 5, it wouldn't raise an error but it really should.

I am thinking that I might add an importable helper function to do the same workaround I posted above so users can call it explicitly in cases that make sense to them.

Ashark commented 10 months ago

So this would not be possible to just use the normal -> Developer instead of -> get_subclasses(Developer)?

Maybe you just make another decorator, or a parameter to your existing decorator, so that it behaves as expected in both cases? Like @type_enforced.Enforcer(subclasses=True)?

connor-makowski commented 10 months ago

I am going to go ahead and close this out with: https://github.com/connor-makowski/type_enforced/pull/28

See the docs here: https://github.com/connor-makowski/type_enforced/blob/1141e6523b3fe34f5be900c543d8cd3815827479/README.md#validate-classes-with-inheritance

Ashark commented 10 months ago

It is unfortunate that you have to edit the code of the functions along with decorator. But thanks for providing this function anyway.

connor-makowski commented 10 months ago

I agree. To be honest, this might change in the next major version, but for backwards compatibility, I couldn't just modify the standard function. I do like the decorator modification, but then I would need to curry the decorator, which is possible and I have written the the code to do it, but it is additional overhead.