laughingman7743/PyAthena

Mypy Error When using Connection.cursor method to instantiate cursor

bboggs-streambit opened this issue · 2 comments

bboggs-streambit commented
image

Minimal Example to recreate:

Create a class which wraps the cursor, type-hinting the specific cursor type you wish to use.

(Type hinting any BaseCursor subclass results in the error):

from typing import Callable
from pyathena.cursor import Cursor

class MyDbWrapperClass:
  database_cursor: Callable[[], Cursor]

  def __init__(self, database_cursor: Callable[[], Cursor] )
      self.database_cursor = database_cursor

my_db_wrapper_instance = MyDbWrapperClass(database_cursor=lambda: Connection(
                            work_group=s.ATHENA_WORKGROUP,
                            region_name=s.ATHENA_REGION,
                            cursor_class=Cursor,
                        ).cursor())

Run mypy on the given file

 mypy .

Result:

my-repo/my-file:linenumber error: Argument "database_cursor" to "MyDbWrapperClass" has incompatible type "Callable[[], BaseCursor]"; expected "Callable[[], Cursor]"  [arg-type]

Cause of error

BaseCursor is type hinted as the return value of the Connection.cursor method in ``

# pyathena.connection
   ....
  def cursor(self, cursor: Optional[Type[BaseCursor]] = None, **kwargs) -> BaseCursor:

Though every instance of a subclass of BaseCursor is an instance of BaseCursor, as an abstract class, BaseCursor isn't instantiable, and if it were, a concrete instance of the BaseCursor could not be used in place of any of the subclasses.

Proposed Solution

Replace BaseCursor with a CursorType TypeVar, which is bound to BaseCursor

Define the TypeVar
# pyathena.connection
from typing import TypeVar
...

CursorType = TypeVar("CursorType", bound=BaseCursor)
Replace BaseCursor with the type var in the type signature for Connection.cursor method and the type hint for the constructor's cursor_class argument
# pyathena.connection
....

class Connection:
    ....
    cursor_class: Type[CursorType] = Cursor,

    def cursor(self, cursor: Optional[Type[CursorType]] = None, **kwargs) -> CursorType:

EDIT:

It appears the solution isn't as straightforward as I thought. Digging in further....

laughingman7743 commented

Pull requests are always welcome.