Detangle the issues with magic arguments

[kiddinn]

ATM when you initialize a magic class it has an optional argument of arguments, which is a list of MagicArgument.

If it is not supplied it is ignored and arguments are derived from a docstring.

This could be detangled, and MagicArgument be better defined and used throughout. Also it should not be done in the __init__ function but arguments should be configured after initialization of the object.

[mariuszlitwin]

I would like to propose major simplification of the current magic framework. Please find some details below.

Definition of Magic

class MagicProtocol(typing.Protocol):
    def __call__(
        data: Text,
        *args:Union[Text, int, bool],
        **kwargs:Union[Text, int, bool]
    ) -> Any:
        pass

Definition of Argument parsers arguments

Argument types would be defined separately for each type. For example argument of type int would be defined as:

class MagicIntArgument:
    flags: List[Text]
    default: Optional[Int]
    help: Text

    def add_to_argparse(self, parser argparse.ArgumentParser):
        # code to transform MagicIntArgument to argparse.ArgumentParser.add_argument invocation
        ...

Similar classes would be defined for other argument types (Text, bool and others as needed).

picatrix_magic decorator signature would stays the same:

def picatrix_magic(
    function: Optional[MagicProtocol] = None,
    arguments: Optional[List[MagicArgument]] = None,
    name_func: Optional[Callable[[Text], Text]] = None,
    conditional: Optional[Callable[[None], bool]] = None
) -> MagicProtocol:
    ...

If arguments is provided, it will be used to parse magic arguments and provide them to decorated function.
If it's not provided, arguments will get parsed out of decorated function definition

def _magic_to_arguments(magic: MagicProcotol) -> List[MagicArgument]:
    hints = typing.get_typing_hints(magic)
    ...

Rest of the code will stay more or less the same, but will follow the path in which arguments is provided - either explicitly by caller or parsed out of magic definition.

Concerns:

@bladyjoker, @kiddinn - I would like to ask your opinion on following topics:

1. Magic return type
   Any type is a broad one and kind of defeats a purpose of having a type at all. Initially I was thinking about making it pd.DataFrame, but then I saw that Timesketch magics are using Dict and other types as output. Do you see any way to define all possible valid outputs of the Magic?
2. Generic MagicIntArgument
   Implementation that I proposed above is serving the purpose, but is very repetitive and inflexible. I think it can be improved through use of typing.Generic. Do you happen to have an experience with Python generics and can share your opinion here? Alternatively - can you suggest any environment where I can quickly check my assumptions about typing? My usual development cycle is to validate assumptions in REPL and then code the most succesful solution. Unfortunately type verification is disabled in runtime.
3. I would like to use https://pypi.org/project/docstring-parser/ for parsing docstrings and extracting descriptions of arguments. In the future I would like to also use it to enable more end-user-friendly functools.partials (reformatting docstrings to take into the account that some attributes got already applied by partial). Do you have any objections against this dependency?

[kiddinn]

ok, to answer your questions here

1. Return type. It is hard to predict the future need of a return type, often times it is a DataFrame, but it can be a dict, it can be a single value, int, string, etc... it can also be none... as in timesketch_set_active_sketch which doesn't produce an output. You may want to write a magic that returns a bool to indicate whether it was successful or not, that's the reason for teh Any, I agree it is not ideal, and it would be good to have something else, but I'm not sure what we can do here without restricting the magics to a limited set/usability.

2. We need to start adding more typing verification... we can use tools like mypy to automatically run against the codebase (I've been experimenting with it, but I need to make some changes to have it work in the first place). This could run on every PR, just like pylint and unit tests, to make sure the codebase is enforcing typing. My idea is also to get rid of the docstring parsing to enforce the typing and use the build-int typing to do the casting and enforcing of arguments.

3. I don't have any objections about that.

I was also thinking, do we need to have the magic arguments at all, or just have the typing in the functions and parsing of docstrings? Do we need the added flexibility but still a lot more complexity by also supporting defining these MagicArguments?