Skip to content

Commit

Permalink
Use Ruff and enforce full type annotation coverage (#60)
Browse files Browse the repository at this point in the history
I got here because I was trying to get rid of some `no-untyped-call` mypy errors internally, which were because our `__init__` method doesn't have annotations. To fix it thoroughly:

- Use Ruff to enforce full annotation coverage
- Also replace other linting tools (pyupgrade, pycln, isort, flake8) so we have fewer tools to worry about
- Update the project URL because I noticed it
- Fix any new errors
  • Loading branch information
JelleZijlstra authored Jan 24, 2024
1 parent cc429cf commit 3bc6943
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 46 deletions.
30 changes: 4 additions & 26 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
repos:
# Order matters because pyupgrade may make changes that pycln and black have to clean up
- repo: https://github.com/asottile/pyupgrade
rev: v3.8.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.14
hooks:
- id: pyupgrade
args: [--py37-plus]

- repo: https://github.com/hadialqattan/pycln
rev: v2.1.5
hooks:
- id: pycln
args: [--config=pyproject.toml]

- repo: https://github.com/pycqa/isort
rev: 5.12.0
hooks:
- id: isort
name: isort (python)
- id: ruff
args: [--fix]

- repo: https://github.com/psf/black
rev: 23.3.0
Expand All @@ -30,15 +17,6 @@ repos:
- id: end-of-file-fixer
- id: trailing-whitespace

- repo: https://github.com/pycqa/flake8
rev: 6.0.0
hooks:
- id: flake8
additional_dependencies:
- flake8-bugbear
- flake8-comprehensions
- flake8-simplify

- repo: https://github.com/pre-commit/mirrors-prettier
rev: v3.0.0-alpha.9-for-vscode
hooks:
Expand Down
46 changes: 39 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ dependencies = [
]

[project.urls]
"Homepage" = "https://github.com/quora/poe-protocol"
"Homepage" = "https://developer.poe.com/"

[tool.pyright]
pythonVersion = "3.7"
Expand All @@ -38,10 +38,42 @@ pythonVersion = "3.7"
target-version = ['py37']
skip-magic-trailing-comma = true

[tool.pycln]
all = true
[tool.ruff]
select = [
"F",
"E",
"I", # import sorting
"ANN", # type annotations for everything
"C4", # flake8-comprehensions
"B", # bugbear
"SIM", # simplify
"UP", # pyupgrade
"PIE810", # startswith/endswith with a tuple
"SIM101", # mergeable isinstance() calls
"SIM201", # "not ... == ..." -> "... != ..."
"SIM202", # "not ... != ..." -> "... == ..."
"C400", # unnecessary list() calls
"C401", # unnecessary set() calls
"C402", # unnecessary dict() calls
"C403", # unnecessary listcomp within set() call
"C404", # unnecessary listcomp within dict() call
"C405", # use set literals
"C406", # use dict literals
"C409", # tuple() calls that can be replaced with literals
"C410", # list() calls that can be replaced with literals
"C411", # list() calls with genexps that can be replaced with listcomps
"C413", # unnecessary list() calls around sorted()
"C414", # unnecessary list() calls inside sorted()
"C417", # unnecessary map() calls that can be replaced with listcomps/genexps
"C418", # unnecessary dict() calls that can be replaced with literals
"PERF101", # unnecessary list() calls that can be replaced with literals
]

ignore = [
"B008", # do not perform function calls in argument defaults
"ANN101", # missing type annotation for self in method
"ANN102", # missing type annotation for cls in classmethod
]

[tool.isort]
profile = "black"
combine_as_imports = true
skip_gitignore = true
line-length = 100
target-version = "py37"
22 changes: 14 additions & 8 deletions src/fastapi_poe/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import os
import sys
import warnings
from typing import AsyncIterable, BinaryIO, Dict, Optional, Union
from typing import AsyncIterable, Awaitable, BinaryIO, Callable, Dict, Optional, Union

import httpx
from fastapi import Depends, FastAPI, HTTPException, Request, Response
Expand All @@ -15,6 +15,7 @@
from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer
from sse_starlette.sse import EventSourceResponse, ServerSentEvent
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.types import Message

from fastapi_poe.types import (
AttachmentUploadResponse,
Expand Down Expand Up @@ -43,15 +44,17 @@ class AttachmentUploadError(Exception):


class LoggingMiddleware(BaseHTTPMiddleware):
async def set_body(self, request: Request):
async def set_body(self, request: Request) -> None:
receive_ = await request._receive()

async def receive():
async def receive() -> Message:
return receive_

request._receive = receive

async def dispatch(self, request: Request, call_next):
async def dispatch(
self, request: Request, call_next: Callable[[Request], Awaitable[Response]]
) -> Response:
logger.info(f"Request: {request.method} {request.url}")
try:
# Per https://github.com/tiangolo/fastapi/issues/394#issuecomment-927272627
Expand All @@ -75,8 +78,9 @@ async def dispatch(self, request: Request, call_next):
return response


async def http_exception_handler(request, ex):
async def http_exception_handler(request: Request, ex: Exception) -> Response:
logger.error(ex)
return Response(status_code=500, content="Internal server error")


http_bearer = HTTPBearer()
Expand Down Expand Up @@ -139,7 +143,7 @@ async def on_error(self, error_request: ReportErrorRequest) -> None:
logger.error(f"Error from Poe server: {error_request}")

# Helpers for generating responses
def __init__(self):
def __init__(self) -> None:
self._pending_file_attachment_tasks = {}

async def post_message_attachment(
Expand Down Expand Up @@ -232,7 +236,9 @@ async def _make_file_attachment_request(
logger.error("An HTTP error occurred when attempting to attach file")
raise

async def _process_pending_attachment_requests(self, message_id):
async def _process_pending_attachment_requests(
self, message_id: Identifier
) -> None:
try:
await asyncio.gather(
*self._pending_file_attachment_tasks.pop(message_id, [])
Expand Down Expand Up @@ -434,7 +440,7 @@ async def index() -> Response:
)

@app.post("/")
async def poe_post(request: Request, dict=Depends(auth_user)) -> Response:
async def poe_post(request: Request, dict: object = Depends(auth_user)) -> Response:
request_body = await request.json()
request_body["http_request"] = request
if request_body["type"] == "query":
Expand Down
6 changes: 3 additions & 3 deletions src/fastapi_poe/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@
from .types import (
ContentType,
Identifier,
MetaResponse as MetaMessage,
PartialResponse as BotMessage,
ProtocolMessage,
QueryRequest,
SettingsResponse,
ToolCallDefinition,
ToolDefinition,
ToolResultDefinition,
)
from .types import MetaResponse as MetaMessage
from .types import PartialResponse as BotMessage

PROTOCOL_VERSION = "1.0"
MESSAGE_LENGTH_LIMIT = 10_000
Expand Down Expand Up @@ -274,7 +274,7 @@ async def _load_json_dict(
{"data": data, "message_id": message_id},
)
# If they are returning invalid JSON, retrying immediately probably won't help
raise BotErrorNoRetry(f"Invalid JSON in {context!r} event")
raise BotErrorNoRetry(f"Invalid JSON in {context!r} event") from None
if not isinstance(parsed, dict):
await self.report_error(
f"Expected JSON dict in {context!r} event",
Expand Down
3 changes: 1 addition & 2 deletions tests/test_types.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import pydantic
import pytest

from fastapi_poe.types import PartialResponse


def test_extra_attrs():
def test_extra_attrs() -> None:
with pytest.raises(pydantic.ValidationError):
PartialResponse(text="hi", replaceResponse=True) # type: ignore

Expand Down

0 comments on commit 3bc6943

Please sign in to comment.