Skip to content

Expose Value type explicitly #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jsurany opened this issue Sep 23, 2024 · 0 comments · Fixed by #79
Closed

Expose Value type explicitly #78

jsurany opened this issue Sep 23, 2024 · 0 comments · Fixed by #79

Comments

@jsurany
Copy link

jsurany commented Sep 23, 2024

Is your feature request related to a problem? Please describe.
type checking tooling doesn't do well in assigning the types of dictionaries when its values take on more than one type.

parameters = {"foo": "bar", "baz": 1}  # mypy thinks this is dict[str, object]
_ = cursor.execute(query, parameters=parameters)  # mypy complains about this

so the user has to manually input types

parameters: dict[str, str | int] = {"foo": "bar", "baz": 1}  # mypy knows this is dict[str, str | int]
_ = cursor.execute(query, parameters=parameters)  # mypy is ok with this

but the actual type for parameters in this interface is defined by an alias, Value (from cdb2.py), and it allows lots of different types. Setting types every time for a complex interface isn't ergonomic. It'd make sense to be able to do something like

from comdb2.dbapi2 import Value

parameters: dict[str, Value] = {"foo": "bar", "baz": 1}
_ = cursor.execute(query, parameters=parameters) 

but Value isn't explicitly exported by any module, so mypy (rightly) complains about using it. We need to explicitly export the type in order for mypy to accept its usage. This makes the type part of the public-facing interface from this package, but I think that's desirable - I also think it's generally correct to publicly expose any types that are used to define public interfaces/functions.

Describe the solution you'd like
export the Value type so that it can be used for type checking in client code.

Describe alternatives you've considered
We could try to make it so mypy correctly interprets union-value dictionaries instead of assigning the general abstraction dict[str, object], but I don't think that kind of change is tractable? It feels like that's a deliberate simplification on the part of the tool.

We could also just bite the bullet and write out the full abstraction every time (my current solution), but this isn't ergonomic for the developer.

Additional context
N/A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant