Skip to content

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?

@pmeier pmeier dismissed their stale review October 27, 2022 16:07

Path shouldn't be in the annotations

@frgfm frgfm requested review from NicolasHug and pmeier and removed request for NicolasHug and pmeier 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