Use @types decorator to enforce arguments types

[RemDelaporteMathurin]

I spotted this decorator below. Thanks @EmilyBourne for sharing this repo 😄 https://github.com/EmilyBourne/Pygyro/blob/02031f96f9b05c3c523290fe349eeef9e39138ba/pygyro/advection/accelerated_advection_steps.py#L21.

We could make use of this decorator to enforce the arguments types. This could greatly reduce the amount of code in all the setters.

More doc: https://stackoverflow.com/questions/47298825/how-to-get-argument-type-hint-in-decorator

[shimwell]

Great idea Remi. I like the idea of writing a decorator that access the type hints of a function and check all the arguments have the correct type.

[EmilyBourne]

This particular decorator actually come from github.com/pyccel/pyccel it only enforces the type if the code is accelerated. But a decorator that does what you're describing sounds like a good idea. My code is also full of

if not isinstance(arg, TYPE):

statements

[RemDelaporteMathurin]

Ok so we'd have to describe our own custom decorator. Sounds good to me

[shimwell]

This could be used as an @decorator but it falls appart when it comes to Optional[] and other more complex args.

from inspect import signature
from typing import get_type_hints

def check_input_args_types_against_type_hints(f):
    def decorated(*args, **kwargs):
        type_hints = get_type_hints(f)
        keyword_arg_dict = signature(f).bind(*args, **kwargs).arguments            
        counter=0
        for name, value in keyword_arg_dict.items():
            if name is not 'self':  # allows use with class methods
                if not isinstance(value, type_hints[name]):
                    msg = 'Input argument for {} is not of type {}'.format(name, type_hints[name])
                    raise TypeError(msg)
                counter+=1
        return f(*args, **kwargs)
    return decorated

I'm looking into typeguard at the moment

[shimwell]

I had a quick go at using typeguard on this branch https://github.com/fusion-energy/paramak/tree/decorator_type_hinting_combo

Adding @typechecked to the Shape class:

Running python and then importing the paramak package now prints quite a lot of type check missing errors. This can be avoided by running python in -O mode but I don't think users will do that and I think the number of warnings is quite high.

So further work on adding type hinting is needed before enabling typeguard to the Shape class

I have instead added @typechecked to several methods in the shape class which provides some checking

[RemDelaporteMathurin]

Does it mean we can't use this for classes? or that we need to extend the @typechecked decorator?

Do we even need the Optional thing? Here it seems to be working without: https://stackoverflow.com/questions/38727520/how-to-add-default-parameter-to-functions-when-using-type-hint

In Shape() for instance, we don't use Optional for points even though it is a default parameter: https://github.com/fusion-energy/paramak/blob/3e29d313ca183fb0f0600c7d45aeffadd46bd4f4/paramak/shape.py#L90-L91

Sorry my type hinting skills are a bit rusty as you know

[shimwell]

The @TypeChecked is great for classes, just our classes need some more work before they are ready (more type hinting needed)

optional also union and I guess List and Tuple when they have specified types for the elements within them are tricky to get working in a custom decorator like the one I have a few comments up.

So I think @TypeChecked is the way to go but just a gradual phase in while more type hints are added.

There are a few cases where adding type hints is tricky and results in a circular import so something might need to be mixed in there.

[RemDelaporteMathurin]

My question was more "Why do we need Optional for considering we don't use it for all optional arguments?"

Isn't there a way to make it work with typechecked instead of creating our own? surely we're not the first to have several acceptable types for one argument right?

[shimwell]

Optional does work with typechecked. I guess ideally we would have Optional on all optional arguments.

[RemDelaporteMathurin]

Ah! Sorry I don't quite get what doesn't work then. Is there even anything that doesn't work? 😶

[shimwell]

The custom decorator that I wrote doesn't work with Optional. But that is ok as we don't need to use it

[RemDelaporteMathurin]

Ah ok gotcha! So we need to do some work in order to add the type check in the missing places, am I right?

[shimwell]

Yep, bit of work needed otherwise we will have a few page of warnings printing every time we import paramk.

But good news is that typechecking can be gradually phased in, it doesn't have to be all or nothing.

Also I should mention that this slows the code down a bit. So to run it quickly (with asserts disabled) one must use python -O filename

[RemDelaporteMathurin]

Ok cool! Regarding the performances, it only slows the code down if we add this on top of the manual checking in the setters right? assuming we get rid of these at some point it should be fine

[shimwell]

Perhaps Pydantic is another option https://pydantic-docs.helpmanual.io/usage/validation_decorator/