[spark] Add text2text generation #2506
Conversation
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2506 +/- ##
============================================
+ Coverage 72.08% 73.69% +1.60%
- Complexity 5126 6913 +1787
============================================
Files 473 685 +212
Lines 21970 30203 +8233
Branches 2351 3122 +771
============================================
+ Hits 15838 22258 +6420
- Misses 4925 6429 +1504
- Partials 1207 1516 +309
... and 353 files with indirect coverage changes 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 in Codecov by Sentry. |
| :param dataset: input dataset | ||
| :return: output dataset | ||
| """ | ||
| if not self.model_url and not self.model_name: |
There was a problem hiding this comment.
Like: if not any([self.model_url, self.model_name])?
There was a problem hiding this comment.
Basically xor. Must have one or the other but not both. bool(self.model_url) ^ bool(self.model_name) should work.
There was a problem hiding this comment.
I was assuming both should be fine. Just model_url has higher priority than model_name.
There was a problem hiding this comment.
It is fine, but not great. For cases like this, I tend to ask whether a user would expect that this is the behavior of the API. If they did provide both, would they expect that priority? Instead, an error is clearer because having both properties when only a single one is used already means something is not right. The error will highlight that for the user and ensure they can clarify the situation rather then risk behaving in ways that they don't expect
| :param dataset: input dataset | ||
| :return: output dataset | ||
| """ | ||
| if not self.model_url and not self.model_name: |
There was a problem hiding this comment.
It is fine, but not great. For cases like this, I tend to ask whether a user would expect that this is the behavior of the API. If they did provide both, would they expect that priority? Instead, an error is clearer because having both properties when only a single one is used already means something is not right. The error will highlight that for the user and ensure they can clarify the situation rather then risk behaving in ways that they don't expect
Description
Brief description of what this PR is about
Add text2text generation, see example: deepjavalibrary/djl-demo#331