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 typing annotations to datasets/_optical_flow #6845

Merged
merged 5 commits into from
Dec 12, 2022

Conversation

frgfm
Copy link
Contributor

@frgfm frgfm commented Oct 27, 2022

Following up on #2025, this PR adds typing annotations in models/datasets/_optical_flow

Any feedback is welcome!

cc @datumbox @pmeier @oke-aditya

@pmeier pmeier self-requested a review October 27, 2022 11:15
pmeier
pmeier previously approved these changes Oct 27, 2022
Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks Francois, LGTM! One question though that is more directed at @NicolasHug: given that we are talking mostly about private methods here, is there a benefit of allowing Union[str, pathlib.Path] over a plain Path or str?

torchvision/datasets/_optical_flow.py Outdated Show resolved Hide resolved
torchvision/datasets/_optical_flow.py Outdated Show resolved Hide resolved
@pmeier pmeier dismissed their stale review October 27, 2022 16:07

Path shouldn't be in the annotations

@frgfm frgfm requested review from pmeier and NicolasHug and removed request for pmeier and NicolasHug November 1, 2022 15:00
@frgfm frgfm requested review from NicolasHug and removed request for pmeier December 10, 2022 15:49
Copy link
Collaborator

@pmeier pmeier 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 Francois!

@pmeier pmeier removed the request for review from NicolasHug December 12, 2022 12:53
@pmeier pmeier merged commit 93723b4 into pytorch:main Dec 12, 2022
@github-actions
Copy link

Hey @pmeier!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@frgfm frgfm deleted the typing-opticalflow branch December 12, 2022 20:14
facebook-github-bot pushed a commit that referenced this pull request Dec 16, 2022
Summary:
* style: Added typing annotations to datasets/_optical_flow

* style: Reverted back to str typing

Reviewed By: YosuaMichael

Differential Revision: D42046594

fbshipit-source-id: 0a6b4720602a2bd83457733824205f66839ae2c1

Co-authored-by: Philip Meier <github.pmeier@posteo.de>
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.

4 participants