Skip to content

This library should have type annotations #795

Description

@benbariteau
No description provided.

Activity

  1. rnestler commented on Aug 15, 2022

    @rnestler
    Contributor

    There are stubs available here: https://pypi.org/project/types-babel/

  2. lukasjuhrich commented on Sep 23, 2022

    @lukasjuhrich
    Contributor

    There are stubs available here: https://pypi.org/project/types-babel/

    I'm not sure whether that's what you meant to imply, but these stubs pretty much insufficient, as most method signatures in there are untyped.

    Question for the maintainers: The type hints would need to be python3.6 compatible, as that's the lowest advertised python version in the setup.py, correct?

  3. akx commented on Sep 23, 2022

    @akx
    Member

    @lukasjuhrich Yes, I'd say at least one more version should be compatible with Python 3.6, if possible (even if it is EOL already).

    I wonder if there's a tool that could automagically merge those type stubs to our source here...

  4. lukasjuhrich commented on Sep 23, 2022

    @lukasjuhrich
    Contributor

    @akx that wouldn't be really helpful as the aforementioned stubs are just the autogenerated ones in typeshed, see for instance the content of babel/support.pyi. They're all either untyped or annotated with Any.

  5. akx commented on Sep 23, 2022

    @akx
    Member

    Well - whatever isn't any-typed, of course.

    Monkeytype has proven an useful tool for generating initial typing from test runs, too.

  6. lukasjuhrich commented on Sep 23, 2022

    @lukasjuhrich
    Contributor

    Well - whatever isn't any-typed, of course.

    Oh, I didn't look far enough. You're right, other modules contain nontrivial hints.

    I wonder if there's a tool that could automagically merge those type stubs to our source here...

    There is (was?) retype, but I've never used it and it claims to be unsupported as of August 14th.

  7. lukasjuhrich commented on Sep 23, 2022

    @lukasjuhrich
    Contributor

    Okay, so I tried the following on my fork:

    1. pip monkeytype pytest-monkeytypep
    2. Run the tests with pytest --monkeytype-output=./monkeytype.sqlite3 tests, which took ~1500s and produced a 1.2G sample file
    3. monkeytype apply babel.support(see diff) to get an example module I started type hinting merged
    4. Run darker to automatically format lines which are overly long due to the diff nicely.

    I've noticed the following things which required manual intervention:

    1. Some imports are janky. Especially the existence of methods named datetime or date shadow imports which breaks the type annotations; one has to rewrite them to from datetime import datetime as std_datetime or similar.
    2. Many types are too strict: for instance,
      • parameters which are only instantiated as None are typed as None, which is clearly wrong
      • parameters like time are only instantiated with datetimes in tests, while they clearly also accept times as well
    3. The imports are imported in a from typing import […]-style, and I didn't find a way to configure it differently.
      Personally I would recommend a import typing as t as it strikes a nice balance between
      • having to touch imports when changing signatures and thus unnecessary diff headache
      • having too much visual noise in the signature due to arg: typing.Optional[typing.Dict[…]]-like constructs where the typing qualifier just becomes more annoying than anything else

    So at the end of the day one has to carefully go through the modules manually (and, of course, at least validate the annotations' relative consistency by running mypy --check-untyped-defs --follow-imports=silent).
    I think especially point 2 is tricky because ideally the type hints of arguments would be as weak as possible (e.g. not demanding list[T] when you're only iterating over the argument).

    All of this seems to me that an incremental module-by-module approach with mypy checking every step of the way would be more viable than applying all monkeytype hints at once, then reviewing all of that while probably missing a lot of things, and having a giant PR which takes ages to review and merge.

  8. rnestler commented on Mar 2, 2023

    @rnestler
    Contributor

    I think it is important to check type signatures with mypy during CI if a library announces type annotation support via py.typed.

  9. akx commented on Mar 2, 2023

    @akx
    Member

    @rnestler Yes and no. The external typing of a library may be sound, but Mypy can still complain (and currently, it does, a lot), about what the library is doing internally.

  10. rnestler commented on Mar 2, 2023

    @rnestler
    Contributor

    @akx I'd still add it to CI even if you need to have a lot of exceptions defined for it.

  11. akx commented on Mar 3, 2023

    @akx
    Member

    @akx I'd still add it to CI even if you need to have a lot of exceptions defined for it.

    (master) $ mypy babel
    Found 268 errors in 17 files (checked 25 source files)
    

    If by "a lot of exceptions" you mean adding about 268 # type: ignore comments, I don't think that's a good way to go about this.

  12. rnestler commented on Mar 3, 2023

    @rnestler
    Contributor

    If by "a lot of exceptions" you mean adding about 268 # type: ignore comments, I don't think that's a good way to go about this.

    No I meant to ignore some files and rules:

    [tool.mypy]
    exclude = [
        'babel/localtime/_fallback.py',
        ...
    ]

    https://github.com/pallets-eco/flask-caching/blob/master/setup.cfg#LL83-L96C19

    But I guess you're right: With that huge amount of type errors it may be to tedious to add all the exceptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions