Add Type hints to query.py

[Caiofcas]

Starts the process of adding type hints, as proposed in #1533 .

As this is the first PR in that sense some more general things are also added here, such as the type_check section in noxfile.py. The implementation of it was based on https://sethmlarson.dev/tests-arent-enough-case-study-after-adding-types-to-urllib3 and linked issues in that blog post (specifically: urllib3/urllib3#1924 (comment)). The filtering of mypy errors allows us to use the --strict flag while also implementing type hints incrementally.

One other change in noxfile.py is adding the E704 code to the ignores list, as this conflicts with one line @overload def statements, which is the default way black formats them (PyCQA/pycodestyle#1036 and PyCQA/flake8#1921). To me it seems harmless enough but don't know if this is the preferred solution.

For the actual typing, there is one change that is worth mentioning in more detail:

In query.py::Q, the function performed a check to see if name_or_query was an instance of collections.abc.Mapping, afterwards calling the methods copy and popitem on it. Both of these methods are not present on the Mapping base class, with the copy method not present in any collection.

I changed the call to .copy() to a call to copy.deepcopy from the standardlib, and changed the check to collections.abc.MutableMapping since this is the class that defines the .popitem() module.

[cla-checker-service]

💚 CLA has been signed

[miguelgrinberg]

@Caiofcas Awesome, thank you so much! I will review this over the next few days.

[miguelgrinberg]

Looks good overall, but I have a couple of notes/comments:

• Note that I merged main into your branch. This is to get a fix I just added for a flaky test that was causing failures in this PR.
• Would it make sense to add type hints to the query tests as well, or were you planning to do those separately once more of the library is typed?
• I suppose we also want to run the type check as part of the CI?
• This is probably for later, but I'd like to figure out a way to have better typing than Any for the kwargs. I will discuss this internally, it would be nice if we could somehow import these from the appropriate functions or methods in the low-level client. This would be a significant effort that should be handled separately from this, but I think once we start adding types people are not going to be happy that these are missing.

[Caiofcas]

Looks good overall, but I have a couple of notes/comments:

• Note that I merged main into your branch. This is to get a fix I just added for a flaky test that was causing failures in this PR.

Thanks! was confused about what was the failure.

• Would it make sense to add type hints to the query tests as well, or were you planning to do those separately once more of the library is typed?

Yeah, had a look and it was not too much work to add them, so it probably makes sense to add them in this PR. The only thing is I had to add some "looser" type annotations to DslBase.to_dict and functions.SF due to their use in the tests. Also added a flag in mypy.ini so the reexport of SF doesn't have to be commented everytime.

Don't know what the opinion of you guys is on these 'looser' but temporary annotations in exchange for a more reviewable PR, will defer to your judgement.

• I suppose we also want to run the type check as part of the CI?

Added it.

• This is probably for later, but I'd like to figure out a way to have better typing than Any for the kwargs. I will discuss this internally, it would be nice if we could somehow import these from the appropriate functions or methods in the low-level client. This would be a significant effort that should be handled separately from this, but I think once we start adding types people are not going to be happy that these are missing.

Yeah, I don't like it as well but it did seem like to much to try to tackle now, since it would require more precise typing of DslBase and some of the baser classes like that. I think the Any allows people to handle it sufficiently well in their own checks.

[miguelgrinberg]

@Caiofcas I tested the type checking on Python 3.8, which is the oldest we support, and this is the output:

❯ nox -s type_check
nox > Running session type_check
nox > Creating virtual environment (virtualenv) using python3.8 in .nox/type_check
nox > python -m pip install mypy '.[develop]'
nox > Session type_check aborted:
elasticsearch_dsl/query.py:234: error: "list" is not subscriptable, use "typing.List" instead  [misc].

I think this isn't a huge deal because it is an internal part of the library. But if this was a public declaration it would be. What do you think about switching the type check task to use 3.8 so that we don't inadvertently start using newer typing constructs that will not work on the older Pythons? I guess we can also go for a matrix, but I don't believe I've seen other projects do that.

Other than that I really have nothing more to suggest, so we can go ahead and merge this.

[Caiofcas]

@miguelgrinberg Yeah, i think it's best to keep it compatible, was trying to keep it that way for the other annotations.

[miguelgrinberg]

Thank you so much!