Adding type hints and mypy #267
Conversation
|
Hello @sanders41, thanks a lot for your PR :) |
|
No problem, no hurry on my end. Sorry it's a big one I couldn't think of a good way to split it up. |
meilisearch/client.py
Outdated
| """Get all indexes in dictionary format. | ||
|
|
||
| Returns | ||
| ------- | ||
| indexes: list | ||
| indexes |
There was a problem hiding this comment.
Some colons are missing sometimes
meilisearch/index.py
Outdated
| return self.http.delete(f'{self.config.paths.index}/{self.uid}') | ||
|
|
||
| def update(self, **body): | ||
| def update(self, **body: Dict[str, Any]) -> "Index": |
There was a problem hiding this comment.
Can we exchange "Index" for 'Index'? It seems to me that it's better for string literals.
meilisearch/index.py
Outdated
| @@ -70,14 +78,9 @@ def update(self, **body): | |||
| self.updated_at = self._iso_to_date_time(response['updatedAt']) | |||
| return self | |||
|
|
|||
| def fetch_info(self): | |||
| def fetch_info(self) -> "Index": | |||
There was a problem hiding this comment.
Here also "Index" to 'Index'
meilisearch/_httprequests.py
Outdated
| @@ -1,20 +1,29 @@ | |||
| import json | |||
| from typing import Any, Callable, Dict, List, Optional, Union | |||
|
|
|||
meilisearch/_httprequests.py
Outdated
| import requests | ||
|
|
meilisearch/index.py
Outdated
| """Create the index. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| uid: str | ||
| uid |
meilisearch/index.py
Outdated
| UID of the index. | ||
| options: dict, optional | ||
| options |
There was a problem hiding this comment.
| options | |
| options: |
meilisearch/index.py
Outdated
| time interval the method should wait (sleep) between requests | ||
|
|
||
| Returns | ||
| ------- | ||
| update: dict | ||
| update |
There was a problem hiding this comment.
| update | |
| update: |
meilisearch/client.py
Outdated
| """Get all indexes in dictionary format. | ||
|
|
||
| Returns | ||
| ------- | ||
| indexes: list | ||
| indexes |
There was a problem hiding this comment.
| indexes | |
| indexes: |
| mypy: | ||
| name: mypy | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v1 | ||
| - name: Set up Python 3.9 | ||
| uses: actions/setup-python@v1 | ||
| with: | ||
| python-version: 3.9 | ||
| - name: Install pipenv | ||
| uses: dschep/install-pipenv-action@v1 | ||
| - name: Install dependencies | ||
| run: pipenv install --dev | ||
| - name: mypy type check | ||
| run: pipenv run mypy meilisearch |
There was a problem hiding this comment.
Thank you for your PR @sanders41!
Can you add the mypy check to bors too?
|
Hi @sanders41! I've just given you write access to this repo if you agree! It will avoid you to fork this project for the next contributions 🙂 Also, your reviews on PR will be taken into account by GitHub! |
|
bors merge |
Closes #264
Unfortunately the methods in the
HttpRequestsclass can return so may different types, I was up to Union[dict[str, Any], List[Dict[str, Any]], List[str], List[int], str, requests.Response] and not done, that I had to go withAnywhich mean mypy wasn't able to check the return types from calls to methods in that class. Since those are the values that are usually returned from methods in theClientandIndexclasses that means mypy wasn't able to check those values. Luckily that class is only supposed to be used internally so code editors and users of the package will still get the benefits from the type hints and mypy. Developers of this package also still get the same benefits.