Skip to content

Update ToDtype to avoid unnecessary to() calls and fixing types on Transform#6773

Merged
datumbox merged 6 commits intopytorch:mainfrom
datumbox:bugfix/todtype
Oct 14, 2022
Merged

Update ToDtype to avoid unnecessary to() calls and fixing types on Transform#6773
datumbox merged 6 commits intopytorch:mainfrom
datumbox:bugfix/todtype

Conversation

@datumbox
Copy link
Copy Markdown
Contributor

@datumbox datumbox commented Oct 14, 2022

As discussed offline this PR:

  • Avoids the unnecessary .to(None) call
  • Fixes the nit on the _transformed_types of Transform to avoid the superfluous features.is_simple_tensor, features._Feature idiom.

Copy link
Copy Markdown
Contributor

@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 Vasilis!

pmeier
pmeier approved these changes Oct 14, 2022
@datumbox
Copy link
Copy Markdown
Contributor Author

datumbox commented Oct 14, 2022

Fun behaviour of defaultdict:

d = defaultdict(lambda: 10)
d.get(1) # None
d[1] # 10
d.get(1) # 10

@datumbox datumbox added code quality and removed bug labels Oct 14, 2022
@datumbox datumbox changed the title Fix ToDtype to avoid errors when a type is not defined. Update ToDtype to avoid unnecessary to() calls and fixing types on Transform Oct 14, 2022
@datumbox datumbox requested a review from pmeier October 14, 2022 14:26
Copy link
Copy Markdown
Contributor

@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, if CI is green. Not making this mistake again 😛

@datumbox
Copy link
Copy Markdown
Contributor Author

The failing test is unrelated (the flaky gaussian), we are good to go.

@datumbox datumbox merged commit e1aacdd into pytorch:main Oct 14, 2022
@datumbox datumbox deleted the bugfix/todtype branch October 14, 2022 14:52
facebook-github-bot pushed a commit that referenced this pull request Oct 17, 2022
…g types on `Transform` (#6773)

Summary:
* Fix `ToDtype` to avoid errors when a type is not defined.

* Nit `(features.is_simple_tensor, features._Feature)` to `(Tensor,)`

* Fixing linter

* Adding comment.

* Switch back to indexing. Python's default dict seems to have a nasty behaviour.

Reviewed By: NicolasHug

Differential Revision: D40427458

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