Skip to content
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

Let utils.to_categorical support different dtypes. #10846

Merged
merged 1 commit into from
Aug 7, 2018

Conversation

xuhdev
Copy link
Contributor

@xuhdev xuhdev commented Aug 4, 2018

Summary

Currently the returned data type of utils.to_categorical is hardcoded. This PR makes it customizable.

Related Issues

#9262

PR Overview

  • [ n ] This PR requires new unit tests [y/n] (make sure tests are included)
  • [ y ] This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)
  • [ y ] This PR is backwards compatible [y/n]
  • [ y ] This PR changes the current API [y/n] (all API changes need to be approved by fchollet)


# Returns
A binary matrix representation of the input. The classes axis
is placed last.
"""
if not dtype:
dtype = K.floatx()
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not make np_utils depend on backend, so please hardcode the default 'float32' in the signature. It's not going to break if there's a mismatch with floatx anyway since we cast Numpy data when feeding it to models.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fchollet Sorry I misunderstood your comments. I'll fix this.

@xuhdev
Copy link
Contributor Author

xuhdev commented Aug 6, 2018

@fchollet I've addressed all comments. Please check again.

Copy link
Collaborator

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

@fchollet fchollet merged commit 63e2046 into keras-team:master Aug 7, 2018
@xuhdev xuhdev deleted the to_categorical branch August 7, 2018 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants