Disallow implicit Any in types

[gabbard]

Originally posted by @berquist in #84

[jamart28]

@gabbard Is this just wanting mypy to disallow implicit Any? If so does this flag work?

  --disallow-any-generics   disallow usage of generic types that do not
                            specify explicit type parameters

[gabbard]

@jamart28 :

I think we want:

--disallow-any-generics ----disallow-any-expr --disallow-any-unimported

I would recommend adding them one-by-one so you can identify which issues come from which source.

[jamart28]

@gabbard So far adding just --disallow-any-generics causes all instances where we don't specify a size for Tuple errors out in mypy. One such fix is to replace such Tuples which seem to be variable or at least large sizes but all one type with Sequence. Would this be the preferred solution?

[gabbard]

@jamart28 : Can you show me an example of such an error?

[jamart28]

preconditions.py:8:

_ClassInfo = Union[type, Tuple[Union[type, Tuple], ...]]

@gabbard

[gabbard]

@jamart28 : That particular example is weird and can be type: ignored. Can you show me another example (or several)?

[jamart28]

Yes let me find one more straightforward of the same type

[jamart28]

Looking back through it seems like these situations may actually have to do with the inside typings (for the one above the inside Tuple has no parameter)
As such this works:

_ClassInfo = Union[type, Tuple[Union[type, Tuple[Any, ...]], ...]]

Is this preferable or is something more specific needed? Furthermore, should I just replace all implicit Any's with an explicit Any or attempt to classify something more specific? @gabbard

[gabbard]

@jamart28 : In the _ClassInfo situation, I think this is fine. I would need to see more examples to comment more generally.

[jamart28]

@gabbard: Of the situation with the dots or all the errors after updating mypy?

[gabbard]

All the errors after updating mypy

[jamart28]

This is just with the first flag above and are all caused by a form of having a generic with no typing after it for what is expected for the generic. In my opinion, the easiest way to show all these would be for me to fix them and then open a draft pull request showing the diffs. I was planning on doing this with each flag above. Would this be acceptable or is a different course of action preferred?

[gabbard]

That is a good approach. If any don't have obvious fixes, link me to the relevant lines of code and I will comment.

[jamart28]

@gabbard: Is there any particular reason we type some variables and not others? I'm specifically referring to a situation like the following lines:

vistautils/vistautils/io_utils.py

Line 232 in e187056

old_close: Callable = ret.close

vistautils/vistautils/io_utils.py

Line 380 in e187056

old_close = ret.close

vistautils/vistautils/io_utils.py

Line 600 in e187056

old_close = ret.close

Since all of these are assigned immediately and not reassigned anywhere in their function, wouldn't it be favorable to a long typing to have no typing (almost simulating an assumed typing from something like kotlin)? Also considering that's basically what's done with the second two lines.

[gabbard]

@jamart28 : In some cases we may have underestimated the type inference, so it is possible some type annotations could be removed without harm.

[jamart28]

@gabbard: So for a quick clarification to hold over until I look at mypy a little closer, does mypy do type inference on variables if possible?

[gabbard]

@jamart28 : mypy attempts to do some degree of type inference, though what it can do is usually more limited than e.g. Kotlin. See https://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html

[jamart28]

@gabbard: Do you see anything in the line below that would match this error: vistautils/key_value.py:169: error: Missing type parameters for generic type?

vistautils/vistautils/key_value.py

Line 169 in e187056

class KeyValueLinearSource(Generic[K, V], AbstractContextManager, metaclass=ABCMeta):

[gabbard]

Odd - my guess would be that AbstractContextManager is generic, though I don't see it documented as such?

Can you see if you can reproduce this error with a minimal example of a generic class deriving from AbstractContextManager?

[gabbard]

Aha, it looks like we should be using typing.ContextManager, which is generic, though it doesn't document exactly what the type of the argument is. I believe it should be:

 class KeyValueLinearSource(Generic[K, V], ContextManager["KeyValueLinearSource"], metaclass=ABCMeta): 

[jamart28]

I can try that.

[jamart28]

Even with that the issue persists and throws all the following errors with it:

[jamart28]

Ok. Doing

class KeyValueLinearSource(Generic[K, V], AbstractContextManager[KeyValueLinearSource[Any, Any]], metaclass=ABCMeta):

gets rid of the Missing type parameters for generic type but again gives all the above errors.

[jamart28]

As such one fix is:

class KeyValueLinearSource(Generic[K, V], AbstractContextManager[Any], metaclass=ABCMeta):

Let me know if this is acceptable.

[gabbard]

What if you use K,V as the args to the inner instance of the KeyValueLinearSource type?

[jamart28]

We just get more specific errors sadly:

[gabbard]

@jamart28 : poke me about this tomorrow and I can help you more. In the meantime, work on other classes of errors.

[jamart28]

I can do that. I'll move over to something else for today as the next issue I will most likely need help with as well since I have the cause of the issue narrowed down but not the why or the fix.

[jamart28]

The following error occurs when running with just the --disallow-any-generics flag for now:
I've traced this to having an issue with the import of immutabledict from immutablecollections. As such one fix would be to trace the error through the import and fix it there, but another is to possibly add a flag to ignore imports (--follow-imports skip) however this seems to throw another error so further research would be needed. Most likely the best solution would be to trace the error through the import and fix it there. If that's the solution wanted I can set up an issue and work it on that repo.

[jamart28]

Found the solution for the first problem (the one @gabbard was originally helping me with). Adding an Any causes an error on actually running the program since the AbstractContextManager is not subscriptable, but mypy wants it to have a typing. As such, the possible solutions are to type ignore (the better in my opinion) or figure out what typing mypy will accept and handling the situation based on type checking like this.

[lichtefeld]

@jamart28 Correct, we should go try to make this fix in the immutablecollections codebase.

I think I'm going to defer to @gabbard on the route for the problem you were originally discussing with him.

[jamart28]

@LichMaster98 The next two flags suggested above produce the following results. I would like to know if these are intended results and thus should be pursued for a fix or if they should not be added:
--disallow-any-expr: This disallows all Any's. As such it targets explicit Any's as well, not allowing Any to be used in any capacity.
--disallow-any-unimported: This one disallows Any's assumed from a type that was imported but not followed by mypy thus causing mypy to assume Any. Realistically the only errors this causes has to do with the following import not being followed in range.py:

vistautils/vistautils/range.py

Line 26 in 0b0be9d

from sortedcontainers import SortedDict

[lichtefeld]

@jamart28

1. I'm unsure if we want to eliminate the ability to Explicitly use Any

2. We might be able to implement the second while just explicitly ignoring the issue on SortedDict.

[jamart28]

1. That was my though process but I wanted to check as it was in the list of flags to check
2. We certainly could I just was unsure if it was necessary. I can definitely throw some # type: ignores in and enable that flag.