Skip to content

Moved pfm file reading into dataset utils #6270

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

Merged
merged 2 commits into from
Jul 14, 2022
Merged

Moved pfm file reading into dataset utils #6270

merged 2 commits into from
Jul 14, 2022

Conversation

TeodorPoncu
Copy link
Contributor

@TeodorPoncu TeodorPoncu commented Jul 14, 2022

Addresses #6259 functionality redefinition both in _optical_flow.py and _stereo_matching.py datasets.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks @TeodorPoncu , LGTM. The only important comment I have is to keep it private as suggested below

@@ -483,3 +484,39 @@ def verify_str_arg(
raise ValueError(msg)

return value


def read_pfm(file_name: str, slice_channels: int = 2) -> np.ndarray:
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep it private, we don't really need to expose it to users

Suggested change
def read_pfm(file_name: str, slice_channels: int = 2) -> np.ndarray:
def _read_pfm(file_name: str, slice_channels: int = 2) -> np.ndarray:



def read_pfm(file_name: str, slice_channels: int = 2) -> np.ndarray:
"""Read file in .pfm format. Might contain
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this sentence is unfinished?

Args:
file_name (str): Path to the file.
slice_channels (int): Number of channels to slice out of the file.
Useful for reading different data formats stored in .pfm files: Optical Flows, Stereo Disparity Maps, etc.
Copy link
Member

Choose a reason for hiding this comment

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

Skipping a line in a parameter description requires to indent it to the right (otherwise sphinx doesn't render it properly):

Suggested change
Useful for reading different data formats stored in .pfm files: Optical Flows, Stereo Disparity Maps, etc.
Useful for reading different data formats stored in .pfm files: Optical Flows, Stereo Disparity Maps, etc.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks @TeodorPoncu ! LGTM when CI is happy

@TeodorPoncu TeodorPoncu merged commit 77940b8 into main Jul 14, 2022
@github-actions
Copy link

Hey @TeodorPoncu!

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

facebook-github-bot pushed a commit that referenced this pull request Jul 21, 2022
Summary:
* Moved pfm file reading into dataset utils

* Made _read_pfm private. Fixed doc format issues.

Reviewed By: jdsgomes

Differential Revision: D37993417

fbshipit-source-id: 15d10ac7dc6a22397ba317b7d7728ba707e812a9
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.

3 participants