postgrest-py
postgrest-py copied to clipboard
`_TableT` is not compatible with `TypedDict`
Bug report
- [x] I confirm this is a bug with Supabase, not with my own application.
- [x] I confirm I have searched the Docs, GitHub Discussions, and Discord.
Describe the bug
https://github.com/supabase/postgrest-py/blob/576a5b84e19c0a379ef10df24f3325c0519d406e/postgrest/_async/client.py#L15-L21
_TableT should be ~~MutableMapping[str, Any]~~ (Edit: Mapping[str, Any]) instead of dict[str, Any] to allow for users to specify a TypedDict as the output type. This seems to be a new-ish regression because my type checker wasn't complaining 4 months ago (edit: this is because there was no _TableT in postgrest 0.19.3 )
Issues from the two main type checkers explaining why dict is not compatible with TypedDict by design
mypy: https://github.com/python/mypy/issues/4976
pyright: https://github.com/microsoft/pyright/issues/6658
To Reproduce
async def f():
db = AsyncClient(supabase_url, supabase_key, options)
response: APIResponse[MessageRecord] = await db.table("messages").select("*").execute()
class MessageRecord(TypedDict):
content: str | None
Expected behavior
Using a TypedDict should be accepted
System information
- OS: Windows
- postgrest-py: 1.0.1
I've had a look at this and even with the change your propose it's still not accepting the new type defined on the returned response. What I've found to work is to not worry about the type at the .execute() level but rather do the type conversion with a Pydantic model by dumping the response to a model.
import os
import asyncio
from typing import Annotated
from dotenv import load_dotenv
from postgrest.exceptions import APIError
from postgrest._async.client import AsyncPostgrestClient
from pydantic import BaseModel, Field
load_dotenv()
url = os.environ.get("SUPABASE_URL", "")
key = os.environ.get("SUPABASE_KEY", "")
class Country(BaseModel):
id: Annotated[int, Field(alias="id")]
name: Annotated[str, Field(alias="name")]
def Countries(data: list):
return [Country(**x) for x in data]
class CountryResponse(BaseModel):
count: Annotated[int | None, Field(alias="count")]
data: Annotated[list[Country], Field(alias="data")]
async def main():
headers = {"Authorization": f"Bearer {key}"}
async with AsyncPostgrestClient(base_url=f"{url}/rest/v1", headers=headers) as client:
try:
resp = (await client.table("countries").select("*").execute()).model_dump()
response = CountryResponse(**resp)
for row in response.data:
print(f"RESPONSE: {row.id} {row.name}")
except APIError as e:
print(f"ERROR: {e}")
asyncio.run(main())
This might not be ideal for you but this is the current working solution I could come up with. I will investigate more to see how we can coerce the type a bit more to follow your original setup.
Seems to be impossible to do inline type hint unless PEP 718 Subscriptable functions is accepted. I checked by reverted to the old version of postgrest that I was using, the reason why it worked before is because there was no _TableT so pyright just assumed Unknown i.e. Any in another name, and allows me to subscript with whatever type I want.
Reason why I believe coercing the type to my original setup is impossible is because
_TableT = Dict[str, Any]
AsyncPostgrestClient.from_(self, table: str) -> AsyncRequestBuilder[_TableT]: ...
has defined already defined the output type through the chain of classes, which passes _TableT into APIResponse[_TableT]
class APIResponse(BaseModel, Generic[_ReturnT]):
data: List[_ReturnT]
"""The data returned by the query."""
and _ReturnT is invariant because data is list[_ReturnT] and the type parameter inside lists is invariant. What is being done right now means APIReponse.data must always be assumed to be Dict[str, Any]. Nothing that is done afterwards short of a cast can matter
Here are 3 ways that I came up that was close to what I wanted
- Split into two lines. I feel like this could work, but I don't understand why it isn't working.
table: AsyncRequestBuilder[MessageRecord] = db.table("messages")
response = await table.select("*").execute()
- Wrapping over AsyncClient/Client to add custom overloads to tables for each known table_name. This still requires
_TableT = Mapping[str, Any]. One small issue is that it is possible to select only a subset of the columns, so the TypedDict has to be total=False
class DatabaseManager(AsyncClient):
"""Wrapper class for the supabase client."""
@overload
def table(self, table_name: Literal["messages"]) -> AsyncRequestBuilder[MessageRecord]: ...
@overload
def table(self, table_name: str) -> AsyncRequestBuilder[Mapping[str, Any]]: ...
@override
def table(self, table_name: str) -> AsyncRequestBuilder[Mapping[str, Any]]:
"""Override the table method to add the correct type hints via overloads."""
return self.postgrest.from_(table_name)
- Hope PEP 718 Subscriptable functions comes out and be able to do, changing
_TableTto aTypeVarwith default
response = await db.table[MessageRecord]("messages").select("*").execute()
Generating a new notification as I finished investigating.
I don't think my approach works with what you guys have without quite a large refactoring and complex typing that includes the default keyword for TypeVar that is only introduced in 3.13.
Since I already have a small fork of postgrest to fix supabase/supabase-py#1207 , I have decided that I will just add a commit that revert the typing and set _TableT = Any.
I would close the issue as not planned, but leaving it to you to decide whether to keep the issue and (eventually) support my usage
TableT had been added a while back - introduced in supabase/postgrest-py#309 . I find it odd that you're only running into this issue now
what you guys have without quite a large refactoring and complex typing that includes the default keyword for TypeVar that is only introduced in 3.13.
@Glinte that was indeed the plan when I'd written that; see the discussion in supabase/postgrest-py#301 for some background
Split into two lines. I feel like this could work, but I don't understand why it isn't working.
This doesn't work because client.from_ explicitly declares that it returns a RequestBuilder[_TableT] (and as TableT = dict[str, Any]); so typehinting as RequestBuilder[MessageRecord] ends up being wrong
The simplest approach I see here is for us to add another method, one that can accept the eventual return type as an argument and then pass it along the query builder chain (you can see I had a similar function in the PR I'd linked here https://github.com/supabase/postgrest-py/issues/301#issuecomment-1721343554)
I was able to get something like that working: After adding:
# _T defined as a plain TypeVar before this use
def typed_from(self, table: str, ret_type: type[_T]) -> SyncRequestBuilder[_T]:
return SyncRequestBuilder[ret_type](self.session, f"/{table}")
(method name to be bikeshedded!), the following works with pyright:
This would be a deviation from the JS client API I guess, but I don't see how it'd be an issue considering it's just a new method being added.
Just thought I'd give my two cents
@anand2312 typed_from is fundamentally wrong (unsound), the earliest possible moment to determine the return type is at select because you can do join selects [1] or not select the whole table. Edit: Oh, I see the way you worked around it by defining the join relationships in the models. That would work yeah. For TypedDicts it would require the user to remember to use total=False though. If possible, it still seems optimal to use type hints and make the type information back-propagate. i.e. the way I originally did it.
response: APIResponse[MessageRecord] = await db.table("messages").select("*").execute()
[1] https://supabase.com/docs/reference/python/select
response = (
supabase.table("users")
.select("name, teams(name)")
.execute()
)
@Glinte right, we're working with the assumption that the type that the user passes in should be representative of the data returned by the entire query (so it should account for any joins and such). Any kind of inference that would reflect that the returned data only has some specific selected columns/dynamically include fields from joined tables etc (both of which would work in typescript, if I'm not wrong) is just not possible currently with Python's typing ecosystem. So yeah, we'd need something like typeddict's with total=False / pydantic models with all fields being optional etc
Type-hinting the final response like you've shown does not back-propagate that type information anywhere; at most it signals to your type checker that this is the type of this variable. But that type annotation is just incorrect, because the type variable T in APIResponse[T] is invariant, so even if TypedDict is type compatible with Mapping[str, Any] the type checker will complain that your typeddict is not the same type as Mapping[str, Any]; here's a simplified example of the situation: pyright playground (edit: re-reading your earlier comment, you already know that haha; I wouldn't hold my breath for PEP 718, I don't think it's gonna get accepted)
The benefit to actually passing the type down the query builder chain like in the example I'd given earlier is that pydantic will use that type to actually validate/coerce the response into that type, so you can be sure that the final object you're getting is of the right type (or pydantic will throw a validationerror)