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

Added missing typing annotations to transforms/functional_tensor #4236

Merged
merged 6 commits into from
Aug 16, 2021

Conversation

frgfm
Copy link
Contributor

@frgfm frgfm commented Jul 31, 2021

Following up on #2025, this PR adds missing typing annotations in transforms/functional_tensor.py. I checked locally by editing the mypy.ini and everything looks good 👍

Any feedback is welcome!

@oke-aditya
Copy link
Contributor

Oh! I too had a shot at this have a look at #4235.
Let's go ahead with either 😄

@frgfm
Copy link
Contributor Author

frgfm commented Jul 31, 2021

Oh! I too had a shot at this have a look at #4235.
Let's go ahead with either

Oh shoot, my bad, I thought I had checked that you hadn't taken care of this one 🙃

@frgfm
Copy link
Contributor Author

frgfm commented Aug 2, 2021

@fmassa I checked locally by removing the mypy ignore from mypy.ini and all tests passed, so I think this is good to go for review :)

@NicolasHug
Copy link
Member

I checked locally by removing the mypy ignore from mypy.ini and all tests passed,

Thanks @frgfm , could you please remove it in this PR as well, so that we can rely on the CI to check the file?

@frgfm
Copy link
Contributor Author

frgfm commented Aug 2, 2021

I checked locally by removing the mypy ignore from mypy.ini and all tests passed,

Thanks @frgfm , could you please remove it in this PR as well, so that we can rely on the CI to check the file?

I would have done it, but there are other files handled by other PRs in transforms. Do you prefer me to remove the ignore and add several for all the other files?

@NicolasHug
Copy link
Member

Yes, we should remove the global ignore of torchvision.transforms.* and manually add the rest of the files which I believe are

_functional_video.py, _transforms_video.py, autoaugment.py, functional.py, functional_pil.py, transforms.py

@frgfm
Copy link
Contributor Author

frgfm commented Aug 3, 2021

I just added other files that were throwing typing errors 👌

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@frgfm
Copy link
Contributor Author

frgfm commented Aug 5, 2021

Ready to be merged @fmassa :)

@datumbox datumbox merged commit 091cbcd into pytorch:master Aug 16, 2021
@github-actions
Copy link

Hey @datumbox!

You merged this PR, but no labels were added.

facebook-github-bot pushed a commit that referenced this pull request Aug 20, 2021
…nsor (#4236)

Summary:
* style: Added missing typing annotations

* style: Fixed typing

* style: Fixed typing

* chore: Updated mypy.ini

Reviewed By: NicolasHug

Differential Revision: D30417193

fbshipit-source-id: 0d0b75c78513e86bd62d13e717d14086fba0916f

Co-authored-by: Francisco Massa <fvsmassa@gmail.com>
Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>
@frgfm frgfm deleted the transforms/functional_tensor branch August 22, 2021 19:10
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.

6 participants