Feature: support ast type hints subscript

[lucasiscovici2]

Hello, and thank you for dev Darglint 🙂 This Pr is intended to allow more complicated types hints to be detected for parameters and return. (With "[") Ex: List[int], Union[int, str], ....

Solution :

• For python < 3.9 astunparse
• For python >=3 .9 ast.unparse

Ex: test.py

def fib(self, a: List[int], b: int) -> List[int]:
        """Fibo.

        Parameters
        ----------
        a : List[str]
            the first number
        b : int
            the second number

        Returns
        -------
        List[str]
            the fibo resut
        """
        return a

original_darglint test.py  
// no error
darglint test.py  
// test.py:fib:0: DAR103:  ~a: expected List[int] but was List[str]
// test.py:fib:0: DAR203:  ~Return: expected List[int] but was List[str]

[lucasiscovici2]

Hi @terrencepreilly , what do you think of this pr ?

[lucasiscovici2]

Hi @terrencepreilly , status ?

[lucasiscovici2]

@benridley why thumbs down ?

[terrencepreilly]

So, I commented on the last PR that you had closed,

Could you explain what problem this PR is solving? How is doing analysis on the function's type signature helpful? Ideally, an explanation for why this is useful would include examples, a brief cost-benefit analysis (e.g. how difficult would this change be to maintain), and an explanation of alternative solutions.

But I never actually got a response. Taking a closer look, I see that you're trying to enable checking more complex type hints. I see why that might be a desired feature. At the same time, checking types described in the docstring against the type hints is only intended for codebases which are transitioning from simply commenting types to proper type hints. So it's a convenience at best. I don't think it warrants adding a dependency to darglint. Especially when I'm not really updating darglint anymore except to accept bug fixes -- it would be hard to guarantee that darglint doesn't break when a dependency breaks.

If you'd like to come up with a solution to extract the information without adding a dependency, I'd be willing to work on it with you. But the first step is to have a clear explanation of the problem, a proposed solution, and an estimate of the work/maintenance that solution would entail.

[lucasiscovici2]

@terrencepreilly Hello and thank you for your answer, I will try to answer your questions as best as I can.

The Darglint goal

Darglint is intended to be "A python documentation linter that checks that the docstring description matches the definition"

The problem

When the type hint is complex darglint ignores the verification with the docstring.

def fib(a: List[str], b: int) -> List[int]:
        """Fibo.

        Parameters
        ----------
        a : Optional[int]
            the first number
        b : int
            the second number

        Returns
        -------
        List[str]
            the fibo resut
        """
        return a

So darglint doesn't see the error for the parameter a (Optional[int] ≠ List[str]). So I conclude that the primary goal of darglint is not accomplished, and that fixing this lack is important.

The solution

• For python < 3.9 astunparse
• For python >=3 .9 ast.unparse

Why this solution

• Do not reinvent the wheel and develop what already exists (which takes time)
• The package have > 100 stars

Estimate of the work/maintenance that solution would entail

• For python >=3.9 we use ast.unparse (standard python library), so normally not too much change
• For python <=3.9 It is a backport of Python standard library ast.unparse, so normally not too much change too (there are only one important file in the package)

[benridley]

I see that you're trying to enable checking more complex type hints. I see why that might be a desired feature. At the same time, checking types described in the docstring against the type hints is only intended for codebases which are transitioning from simply commenting types to proper type hints. So it's a convenience at best.

Maybe I'm missing something, but my main use-case for darglint is ensuring that the type hints I've written actually match the described arguments in the docstring, because as I refactor code I sometimes miss updating the docstrings to the correct types. So being able to validate sum types like Optional[str] is a really important feature for me, and I'm unsure how this is only useful during transitioning... Is there some other method of validation for this kind of use case?

[lucasiscovici2]

@terrencepreilly What do you think about my message ? If you want you can add me as maintainer

[terrencepreilly]

@lucasiscovici-Loreal

Sorry, I was waiting for more. I wrote earlier

If you'd like to come up with a solution to extract the information without adding a dependency...

But your proposed solution adds a dependency. I imagine just having it not work in earlier python version would be fine, if you'd prefer to do that. The ast.unparse function warns that it can throw a recursion error for complex expressions. It would be a good idea, as a one-off, to fuzz test type signatures to see how complex they have to get before that error is raised. I would imagine that any reasonable type signatures wouldn't raise that error, but just in case, it may be a good idea to check.

@benridley

Maybe I'm missing something, but my main use-case for darglint is ensuring that the type hints I've written actually match the described arguments in the docstring

If you're already using type hints, there shouldn't be any reason to document the type in the docstring: it's already available right there in the function signature. Docstrings should be used to capture information which can't be understood from reading the function signature.

[benridley]

@terrencepreilly That's a reasonable point, and I was going to respond saying my organization uses Sphinx to generate documentation from docstrings so we need to include the type information. It seems however we can use https://pypi.org/project/sphinx-autodoc-typehints/ to do this for us, so thanks for prompting me down that path!

[lucasiscovici2]

@terrencepreilly thank you for your answer !