Skip to content
This repository was archived by the owner on Mar 11, 2026. It is now read-only.

Matthew Fix#2406

Merged
bhack merged 8 commits intotensorflow:masterfrom
jonpsy:matthew
Apr 2, 2021
Merged

Matthew Fix#2406
bhack merged 8 commits intotensorflow:masterfrom
jonpsy:matthew

Conversation

@jonpsy
Copy link
Contributor

@jonpsy jonpsy commented Mar 1, 2021

Description

Brief Description of the PR:

Fixes # (issue)
#2339

Type of change

  • Bug fix
  • New Tutorial
  • Updated or additional documentation
  • Additional Testing

Checklist:

  • [ x] I've properly formatted my code according to the guidelines
    • By running Black + Flake8
    • By running pre-commit hooks
  • This PR addresses an already submitted issue for TensorFlow Addons
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • This PR contains modifications to C++ custom-ops

How Has This Been Tested?

The issue created by the user is perfectly solved by this

If you're adding a bugfix or new feature please describe the tests that you ran to verify your changes:

jonpsy added 2 commits March 1, 2021 18:21
- multiclass works for OHE
- fix multiclass test in unit test
@jonpsy jonpsy requested a review from autoih as a code owner March 1, 2021 15:40
@boring-cyborg boring-cyborg bot added the metrics label Mar 1, 2021
@google-cla google-cla bot added the cla: yes label Mar 1, 2021
@bot-of-gabrieldemarmiesse

@marload @autoih

You are owners of some files modified in this pull request.
Would you kindly review the changes whenever you have the time to?
Thank you very much.

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 1, 2021

This should work for one hot encoded label. I'll work on sparse labels as well but I wanted to gather some reviews before. Looking forward! :)

@bhack
Copy link
Contributor

bhack commented Mar 18, 2021

@ZeynepP Can you test this fix?

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 19, 2021

@bhack I've added test case which confirms that patch solves the issue. Would you mind reviewing this patch?

@bhack
Copy link
Contributor

bhack commented Mar 19, 2021

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 19, 2021

Its in the test case :) or do you mean to say I should import sklearn and check their output values? I can do that.

@bhack
Copy link
Contributor

bhack commented Mar 19, 2021

I should import sklearn and check their output values? I can do that.

Yes we already have some tests with sklearn in the repo. I think that you can check these.

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 20, 2021

Done :)

@bhack bhack merged commit e83e71c into tensorflow:master Apr 2, 2021
@jonpsy jonpsy deleted the matthew branch April 2, 2021 09:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants