[proto] Fixes Transform._transformed_types and torch.Tensor #6487
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
vfdev-5
commented
Aug 24, 2022
vfdev-5
commented
- Fixes unexpected behaviour with Transform._transformed_types and torch.Tensor
- Added a test for color space conversion
datumbox
reviewed
Aug 24, 2022
datumbox
reviewed
Aug 25, 2022
pmeier
reviewed
Aug 25, 2022
| @@ -42,7 +42,7 @@ def _transform(self, inpt: Any, params: Dict[str, Any]) -> Any: | |||
|
|
|||
| class ConvertColorSpace(Transform): | |||
| # F.convert_color_space does NOT handle `_Feature`'s in general | |||
| _transformed_types = (torch.Tensor, features.Image, PIL.Image.Image) | |||
There was a problem hiding this comment.
Maybe we should have an alias for these three, since they will often happen together?
There was a problem hiding this comment.
We can do on a follow up. Good idea for when we review the whole input validation mechanism as a whole.
| @@ -45,12 +45,18 @@ def query_chw(sample: Any) -> Tuple[int, int, int]: | |||
| return chws.pop() | |||
|
|
|||
|
|
|||
| def _isinstance(obj: Any, types_or_checks: Tuple[Union[Type, Callable[[Any], bool]], ...]) -> bool: | |||
There was a problem hiding this comment.
Could you also use this in has_all?
There was a problem hiding this comment.
I think we can't change easily for-loops types -> objects into objects -> types.
There was a problem hiding this comment.
True, I didn't see that. Could you add a comment to has_all why we are not using it, since that is not obvious?
|
I'm merging this because I really need this to progress other work. Let's follow up with the non-blocking comments. |
facebook-github-bot
pushed a commit
that referenced
this pull request
Aug 30, 2022
…6487) Summary: * Fixes unexpected behaviour with Transform._transformed_types and torch.Tensor * Make code consistent to has_any, has_all implementation * Fixed failing flake8 check Reviewed By: NicolasHug Differential Revision: D39131010 fbshipit-source-id: 376289d1b0854acdf76f4495a1eabf940058551b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.