Create translator options #2145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportBase: 72.08% // Head: 71.52% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2145 +/- ##
============================================
- Coverage 72.08% 71.52% -0.57%
- Complexity 5126 6337 +1211
============================================
Files 473 628 +155
Lines 21970 27969 +5999
Branches 2351 3008 +657
============================================
+ Hits 15838 20004 +4166
- Misses 4925 6496 +1571
- Partials 1207 1469 +262
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This creates a new tool for TranslatorOptions based on the characteristic function ((InputClass, OutputClass) -> Translator). Unlike the TranslatorFactory, it doesn't require arguments or a model. So, the TranslatorFactory is mainly used as part of model loading where the TranslatorOptions can be more broad. In particular, it connects to both Dataset and Translator. Both of them gain a new function that will return a TranslatorOptions (matchingTranslatorOptions for the Dataset and getExpansions for the translator). Right now, I made both of these optional with a default value of null so that users don't have to define them, it is just a possibility. To connect the TranslatorOptions and TranslatorFactory, I created a class ExpansionTranslatorFactory. This is used for the common abstraction where we have a base translator and multiple "expansions" for it that come before it in pre-processing or after it in post-processing. With a way to construct the base translator with the options, it works as a TranslatorFactory. With a starting Translator, it can become the TranslatorOptions. Lastly and probably worth calling out, the ExpansionTranslatorFactory creates a map of the options. This allows the supported types and the functions to be defined in only one place instead of two. In a followup PR, I will make a version of this that creates a BaseImageTranslatorFactory matching the BaseImageTranslator and will handle all of the repeated code for all of the image translator factories.
frankfliu
force-pushed
the
translatorExpansion
branch
from
edce0a3
to
72ec1e1
Compare
frankfliu
force-pushed
the
translatorExpansion
branch
from
72ec1e1
to
6a38960
Compare
frankfliu
approved these changes
Nov 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This creates a new tool for TranslatorOptions based on the characteristic function ((InputClass, OutputClass) -> Translator). Unlike the TranslatorFactory, it doesn't require arguments or a model. So, the TranslatorFactory is mainly used as part of model loading where the TranslatorOptions can be more broad.
In particular, it connects to both Dataset and Translator. Both of them gain a new function that will return a TranslatorOptions (matchingTranslatorOptions for the Dataset and getExpansions for the translator). Right now, I made both of these optional with a default value of null so that users don't have to define them, it is just a possibility.
To connect the TranslatorOptions and TranslatorFactory, I created a class ExpansionTranslatorFactory. This is used for the common abstraction where we have a base translator and multiple "expansions" for it that come before it in pre-processing or after it in post-processing. With a way to construct the base translator with the options, it works as a TranslatorFactory. With a starting Translator, it can become the TranslatorOptions.
Lastly and probably worth calling out, the ExpansionTranslatorFactory creates a map of the options. This allows the supported types and the functions to be defined in only one place instead of two. In a followup PR, I will make a version of this that creates a BaseImageTranslatorFactory matching the BaseImageTranslator and will handle all of the repeated code for all of the image translator factories.