Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Fix in _convert_to_tensor_if_necessary so that also np.array get converted correctly #649

Merged
merged 6 commits into from
Feb 1, 2022

Conversation

maxilse
Copy link
Contributor

@maxilse maxilse commented Jan 31, 2022

Fix for the _convert_to_tensor_if_necessary method so that PIL.Image as well as np.array get converted to torch.Tensor

The PR also includes a test.

@maxilse maxilse requested review from ant0nsc and vale-salvatelli and removed request for ant0nsc January 31, 2022 15:18
@vale-salvatelli
Copy link
Contributor

Looks good to me! thanks for taking care of this bug

dccastro
dccastro previously approved these changes Jan 31, 2022
Copy link
Member

@dccastro dccastro left a comment

Choose a reason for hiding this comment

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

Approved, just one suggestion for the type-checking.

assert type(batch[SSLDataModuleType.ENCODER][1]) == torch.Tensor
assert type(batch[SSLDataModuleType.LINEAR_HEAD][0]) == torch.Tensor
assert type(batch[SSLDataModuleType.LINEAR_HEAD][1]) == torch.Tensor
assert type(batch[SSLDataModuleType.LINEAR_HEAD][2]) == torch.Tensor
Copy link
Member

Choose a reason for hiding this comment

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

Here it would be safer to use isinstance(...), in case we get a Tensor subclass, for example.

@maxilse maxilse dismissed stale reviews from dccastro and vale-salvatelli via 13396c1 January 31, 2022 17:12
@ant0nsc ant0nsc merged commit b8fe0eb into main Feb 1, 2022
@ant0nsc ant0nsc deleted the maxilse/dataloader_typing_fix branch February 1, 2022 14:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants