Skip to content

fix: client typing changes to accommodate JSON lists#840

Open
jtsextonMITRE wants to merge 8 commits intodevfrom
jtsexton-client-typing-fix
Open

fix: client typing changes to accommodate JSON lists#840
jtsextonMITRE wants to merge 8 commits intodevfrom
jtsexton-client-typing-fix

Conversation

@jtsextonMITRE
Copy link
Collaborator

@jtsextonMITRE jtsextonMITRE commented Apr 25, 2025

Closes #836

@jtsextonMITRE jtsextonMITRE changed the base branch from main to dev April 25, 2025 15:57
@jkglasbrenner
Copy link
Collaborator

This PR was impacted by many of the recent changes to dev, and resolving it looks messy. Just getting this back in sync might require that we carefully reapply changes to a new branch from dev at this point.

More generally, this is still on my radar, but I really want to be careful with how we're solving this and think through whether the approach in this PR is what we actually want. The outcome we want is straightforward, we want to be able to express that the JSON version of the client can return more than just the dict[str, Any] type, since for example list[dict[str, Any]] and list[str | int | float | None] are also valid JSON that json.loads will handle properly. Just using dict[str, Any] | list[Any] | None is not a proper solution because we don't want to force client users to validate the returned type everywhere. The challenge I'm seeing is that the more specific we want to be with our returned types so that we can avoid post-call checks, such that we can say a method returns list[str] rather than list[Any], the more complex it will be to express this using static types and generics. Right now we've tried to address this by adding a second general type everywhere that each method can pick from. However, if we reserve it for something like list[dict[str, Any]], it still excludes list[str | int | float | None], and there can be cases where we'd want the list of scalars like that. I want to make sure that how this is done is expressed at the right level of abstraction, rather than what just seemed to be the quickest patch we could find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] The current JSON client accepts dict[str, Any] only, but should also accept list[dict[str, Any]]

3 participants