omni-us / jsonargparse

Implement minimal boilerplate CLIs derived from type hints and parse from command line, config files and environment variables
https://jsonargparse.readthedocs.io
MIT License
326 stars 49 forks source link

Mypy: `DictComponentsType["_help"]` is not expressed in type declaration #626

Open netanel-haber opened 4 days ago

netanel-haber commented 4 days ago

🐛 Bug report

The type for DictComponentsType doesn't allow the "_help": str entry:

ComponentType = Union[Callable, Type]
DictComponentsType = Dict[str, Union[ComponentType, "DictComponentsType"]]

To reproduce

from jsonargparse import CLI
CLI({"something": {"_help": "Help"}})
> python -m mypy .
...error: Dict entry 0 has incompatible type "str": "str"; expected "str": "Callable[..., Any] | type[Any] | DictComponentsType"
mauvilsa commented 3 days ago

Thank you for reporting! Would you be interested in contributing the fix?

netanel-haber commented 3 days ago

Sure, I'll do so tomorrow

netanel-haber commented 2 days ago

Well, I took a look:

I think adding support for strictly Lireral["_help"]: str to the typing is currently literally impossible in the python typing realm [maybe if and when pep728 is accepted].

We can allow arbitrary string keys at all but the top level, and just asserting at runtime that only literally _help was passed for str: str entries, e.g.:

from typing import Union, Callable, Type, Dict

ComponentType = Union[Callable, Type]
WithStr = Union[ComponentType, str]
NestedComponentsType = Dict[str, Union[WithStr, "NestedComponentsType"]]
DictComponentsType = Dict[str, Union[ComponentType, NestedComponentsType]]

def tryme(components: DictComponentsType) -> None:
    print(components)

tryme({
    "A": {
        "B": "C", # mypy will allow any `str: str`
        "D": lambda: "E"
    }
    #, "F": "G" # mypy throws at top level as expected
})

That will stop mypy from complaining, at the cost of more complex typing, and possible errors that will only be discovered at runtime (non _help entries). I lean against typing improvements if it can incur a runtime penalty.

WDYT? I lean towards "won't fix".

mauvilsa commented 2 days ago

Yes, tricky. I think better to not change anything until there is a more clean way to do it. A bit unfortunate. I did not think about this when I added the _help option.