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.

Sign up for GitHub

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

Jump to bottom

Fix kwargs in crf_decode_forward #2642

Merged
merged 3 commits into from
Jan 21, 2022
Merged

Fix kwargs in crf_decode_forward #2642

merged 3 commits into from
Jan 21, 2022

Conversation

tgsmith61591
Copy link
Contributor

Description

Type of change

  • Bug fix

Checklist:

  • 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?

  • Test to show that when a mask is applied, masked positions are equal to zero
  • Test to show that when masking is not applied, no positions are equal to zero

@boring-cyborg boring-cyborg bot added the text label Dec 30, 2021
@tgsmith61591 tgsmith61591 mentioned this pull request Dec 30, 2021
Closed
@bot-of-gabrieldemarmiesse
Copy link

@howl-anderson

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

@howl-anderson
Copy link
Contributor

Hi @tgsmith61591, I will take a closer look into this PR in the next few days.

@tgsmith61591
Copy link
Contributor Author

Hi @howl-anderson any chance you've gotten to take a look at this? Thanks!

@howl-anderson
Copy link
Contributor

Hi @tgsmith61591, I am working on it, I will finish the PR review ASAP.

howl-anderson
howl-anderson suggested changes Jan 19, 2022
View reviewed changes
Copy link
Contributor

@howl-anderson howl-anderson left a comment

Choose a reason for hiding this comment

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

LGTM, we can merge this PR after @tgsmith61591 make the change according to the comment.

tensorflow_addons/text/tests/crf_test.py Outdated Show resolved Hide resolved
@tgsmith61591
Copy link
Contributor Author

Done @howl-anderson thanks!

howl-anderson
howl-anderson approved these changes Jan 20, 2022
View reviewed changes
Copy link
Contributor

@howl-anderson howl-anderson left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, @seanpmorgan please merge this PR.

@seanpmorgan seanpmorgan merged commit b8cab7f into tensorflow:master Jan 21, 2022
@tgsmith61591 tgsmith61591 deleted the issue_2639 branch January 21, 2022 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@howl-anderson howl-anderson howl-anderson approved these changes

@seanpmorgan seanpmorgan seanpmorgan approved these changes

Assignees
No one assigned
Labels
text
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tgsmith61591 @bot-of-gabrieldemarmiesse @howl-anderson @seanpmorgan @kokoro-team