-
Notifications
You must be signed in to change notification settings - Fork 411
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
Unify preds, target
input arguments for text
metrics [1of2] bert, bleu, chrf, sacre_bleu, wip, wil
#723
Conversation
preds, target
input arguments for text
metrics 1/2preds, target
input arguments for text
metrics 1/2 + nits
preds, target
input arguments for text
metrics 1/2 + nitspreds, target
input arguments for text
metrics 1/2
Codecov Report
@@ Coverage Diff @@
## master #723 +/- ##
=====================================
Coverage 95% 95%
=====================================
Files 170 170
Lines 6759 6769 +10
=====================================
+ Hits 6398 6408 +10
Misses 361 361 |
preds, target
input arguments for text
metrics 1/2preds, target
input arguments for text
metrics [6of12]
@stancld made the metrics as a checkbox also here so you can check progress and could you also update the original issue? |
still WIP? 🐰 |
Yes a bit, I'll need to handle breaking changes add some deprecation warnings as you pointed out in #727 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still need warnings and back compatibility 🐰
Currently adding these... Though, still thinking about if we shouldn't go with |
3a0f7dc
to
317182c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just update the deprecated args in docs, otherwise looks good to me 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just the warning stream, otherwise it looks good
What does this PR do?
This PR unifies
preds, target
input arguments fortext metrics
+ also unifies some minor related stuffPart of #716
Fixes:
bert
bleu
chrf
sacre_bleu
wip
wil
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃