pytypes code is not completely annotated

[jolaf]

The source code of pytypes itself contains no annotations and no stubs are present either.

Therefore mypy issues warnings while checking pytypes-enabled code.

[Stewori]

Adding annotations in pytypes proved disastrous for certain modes, e.g. within a typecheck profiler. It would try to check itself while checking something, ending in an endless typecheck loop. Sure, there are better fixes, e.g. adding @notypecheck to internals. Also, not every internal would be affected. However I used to consider this low priority, especially given the hassle involved. Doesn't mypy have an ignore mode for certain modules?

[jolaf]

Yes, mypy definitely has various controls to work around this, and I totally agree this is low priority.

[jolaf]

Normally type annotations for a library are important for public API, as they help to make sure that the code that uses that API is doing it correctly.

Annotating everything that's not part of public API would only help to verify the correctness of the library itself, and that can be provided by other means and thus is less important.

[dbarnett]

I ran into pretty annoying issues where I ironically lost static type information whenever I tried to enable runtime type checking with the @pytypes.typechecked decorator and things like that. Would it be possible to add a subset of trivial type information to avoid breaking existing declared types like that?

[Stewori]

Would it help for now, i.e. to solve the issue with loosing static type info, to just annotate the decorators according to https://github.com/python/mypy/issues/3157? (Do you know how to do that and would consider to write a PR?) I think the version that just preserves the function signature would be the right one.

Also note that you can apply pytypes' decorators to the whole module which would likely prevent them from being visible as decorators to mypy (did not test this). Also the profiler mode may be an option.

[dbarnett]

Yeah, I think that matches what I had in mind. These would just be changes to add annotations in typechecker.py?

[Stewori]

Yes, at https://github.com/Stewori/pytypes/blob/da056359a8d1dad174316195830a1cb0574893af/pytypes/typechecker.py#L1007. If possible also for override, but I am not sure if that would work well because it takes an optional arg auto.

I further hope that it won't cause issues that typechecked also accepts classes.

I think we don't need tests for this because such tests would involve a static type checker I suppose. It wood be good to have some instructions or example to test it before a release. E.g. in the docstring of typechecked.

[dbarnett]

Okay, I figured out the gist of the changes, but pytype found several type issues in the same file, and I'm having trouble understanding from PEP 561 if adding a py.typed file when the project has type checker errors will cause problems.

Some of these look like legit bugs or easy improvements...

• No type information for backports.typing, which seems to have been deleted by the author anyway. I filed #93 to remove it.
• Some code using possibly-uninitialized None values, like base_argnames at https://github.com/Stewori/pytypes/blob/da056359a8d1dad174316195830a1cb0574893af/pytypes/typechecker.py#L228.
• This line seems to be calling a dict as a function: https://github.com/Stewori/pytypes/blob/da056359a8d1dad174316195830a1cb0574893af/pytypes/typechecker.py#L1144

Please let me know if it's safe to just go ahead and add the py.typed marker in your understanding and I can figure out some of the remaining finer details.

[Stewori]

Thanks for spotting this. _auto_override_modules(mod_name) is supposed to be auto_override_module(mod_name).

Regarding the possibly uninitialized None values, I believe that the code is actually okay, however it is non-trivial to be sure here. I would prefer to leave it as it is until some actual failure pops up. Would it solve the issue to surround the doubtful code by an appropriate try/except to secure the potential None call or index access? Catching TypeError and IndexError should be sufficient (?). I never used pytype and don't know whether it is satisfied by a surrounding try/except. Or would it maybe recognize assert blah is not None as a marker to indicate that it was initialized (in some mystical way)?

[Stewori]

See https://github.com/Stewori/pytypes/commit/d5c08b036efa13ab198a6f2e859771df7ffc2780

[dbarnett]

Yeah, uninitialized None errors are easily resolved with assert, just for some of them I didn't see a reason it was a safe assumption and will probably include a comment above the assertion.

Adding assertions to help the checker (and human readers) follow the invariants is common, not magical at all. The checker is not a serious theorem prover so there are plenty of cases that are guaranteed for reasons it doesn't understand. There's even one it misses right above the base_argnames one because it's initialized in an earlier if block.

[Stewori]

Perfect. Then please feel free to add asserts as required.

[dbarnett]

I was also wondering if we could change the module name case for @typechecked to not return anything. Passing a string for that will sometimes look up and return the module and sometimes return the string unchanged based on several factors. Is there a reason people would be relying on that return value for modules?

[Stewori]

I think the module case does return the module only to be consistent with its behavior in decorator case, where it always returns the input member. I agree it should be defined more explicitly whether a string or module is returned, e.g. always return a module unless the lookup failed, then return None or even raise an error. Or always return the input, i.e. string if it was a string and module if it was a module. I guess this would cause the least trouble for annotating @typechecked as it would always return the type of the input member. I think this would be my favorite solution.

You find it best for type annotation to return always None in module case? I am also fine with that if it breaks no test, but as said above I suspect it would complicate annotation of the generic @typechecked. Also, maybe a confirmation would be good if the lookup succeeded. What do you think of raising an error or warning if the lookup fails?

[dbarnett]

Nah, I wasn't attached to the None idea. Actually I agree making it string→string / module→module unconditionally is intuitive and way simpler for the typing. Seems like you're on board with the idea, so I went ahead and filed #97 for any further discussion and I can send a PR for that shortly.

[Stewori]

This was closed automatically due to https://github.com/Stewori/pytypes/commit/95d58a30a8ddb665500d8e88b13a94e0e0f76373. However, that commit only adds annotations for typechecker.py not for the rest of pytypes. So, I'll reopen since this issue is only partially resolved yet. That said, I should note that I would not require every bit of pytypes to be annotated to close this issue. I think annotating public API (i.e. API imported in pytypes and without leading _) plus maybe some API that has a compelling reason (e.g. all decorators to satisfy mypy) should be sufficient to label this issue as solved.

[dbarnett]

You could reword the issue to "not completely annotated" for clarity if it's going to stay open long.

The only problem for me was pytypes losing type information in projects that use it. I can't think of any big advantages to proactively adding type hints on the rest. Eventually it might help for some IDE features / autocomplete, but for now most tooling like python-language-server doesn't even notice types outside of typeshed.

[Stewori]

I think there's nothing wrong with adding remaining type info (in long term). Last but not least because it serves documentation purposes.