Skip to content
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

Change typing of batch_load_fn to support subclass overriding #27

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

markedwards
Copy link
Collaborator

Using a Protocol to type batch_load_fn does not play well with the documented pattern of overriding the method in a subclass.

For instance:

class TestDataLoader(DataLoader[str, str]):

    async def batch_load_fn(self, keys: list[str]) -> list[str]:
        return [key for key in keys]

This causes mypy to complain Signature of "batch_load_fn" incompatible with supertype "DataLoader".

Fixing by typing batch_load_fn as Callable[[List[KeyT]], Coroutine[Any, Any, List[ReturnT]]], which also avoids the conditional import of Protocol.

@markedwards
Copy link
Collaborator Author

@aawilson, do you have any concerns with this adjustment?

@aawilson
Copy link
Contributor

aawilson commented Nov 14, 2022

Should be fine, I mentioned in the other thread I was surprised at the behavior because we use it (subclassed) with the protocol at work and the linter doesn't mind, I didn't poke at it harder but at the outset want to chalk it up to different linter behavior.

@markedwards markedwards merged commit 3b9d8bf into master Nov 14, 2022
@markedwards markedwards deleted the batch-load-fn-override-typing branch November 16, 2022 07:33
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 this pull request may close these issues.

None yet

2 participants