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

#3337 Add typing overloads to Dataset.__getitem__ for mypy #3382

Merged
merged 9 commits into from
Dec 14, 2021

Conversation

Dref360
Copy link
Contributor

@Dref360 Dref360 commented Dec 4, 2021

Add typing overloads to Dataset.getitem for mypy

Fixes #3337

Iterable
Iterable from collections cannot have a type, so you can't do Iterable[int] for example. typing has a Generic version that builds upon the one from collections.

Flake8
I had to add # noqa: F811, this is a bug from Flake8.

datasets uses flake8==3.7.9 which released in October 2019 if I update flake8 (4.0.1), I no longer get these errors, but I did not want to make the update without your approval. (It also triggers other errors like no args in f-strings.)

src/datasets/arrow_dataset.py Outdated Show resolved Hide resolved
@Dref360
Copy link
Contributor Author

Dref360 commented Dec 6, 2021

Locally the make quality passes with the same dependencies. I would suggest upgrading flake8. (I can take care of it in another PR)
cc @lhoestq

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool thanks ! Let me see if I can fix the flake8 issue

src/datasets/arrow_dataset.py Outdated Show resolved Hide resolved
@Dref360
Copy link
Contributor Author

Dref360 commented Dec 14, 2021

Thank you for fixing flake8! I think we are ready to merge then.

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you !

@lhoestq lhoestq merged commit 047dc75 into huggingface:master Dec 14, 2021
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.

Typing of Dataset.__getitem__ could be improved.
2 participants