Skip to content

Conversation

@TeodorPoncu
Copy link
Contributor

This is a continuation of the PR split (#6311, #6269) which contains the ETH3D low-res two-view dataset.

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 , minor comments but LGTM anyway

disparity_map = np.abs(disparity_map) # ensure that the disparity is positive
mask_path = Path(file_path).parent / "mask0nocc.png"
valid_mask = Image.open(mask_path)
valid_mask = np.asarray(valid_mask).astype(np.bool_)
Copy link
Member

Choose a reason for hiding this comment

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

I was surprised by the us of np.bool_ and looking at numpy's warning, I think this should just be the buildin bool https://numpy.org/devdocs/release/1.20.0-notes.html#using-the-aliases-of-builtin-types-like-np-int-is-deprecated

Comment on lines 2895 to 2897
assert dataset._images and len(dataset._images) == len(
dataset._disparities
), "Training images do not match with training disparities"
Copy link
Member

Choose a reason for hiding this comment

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

Curious why we need this assertion. Isn't it taken care of in the shape_test_... call below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That assertion is unnecessary indeed! Thanks!

@TeodorPoncu TeodorPoncu merged commit 9662001 into main Aug 18, 2022
facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2022
Summary:
* Added ETH3D stereo dataset

* Small doc-reformating

* Removed assertions with no use, changed np conversion

* Added ETH3D stereo dataset

* Removed assertions with no use, changed np conversion

* rebased on main

* Revert "Removed assertions with no use, changed np conversion"

This reverts commit 1478a8c.

* Update to np.bool instead of np.bool_

* lint and mypy nit fix

* test nit

Reviewed By: datumbox

Differential Revision: D39013683

fbshipit-source-id: adf03910e184c63363c5398c171ac668e1b8b3a9
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