-
Notifications
You must be signed in to change notification settings - Fork 0
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
REVAI-3854: SuperApi features #59
Conversation
src/main/java/ai/rev/speechtotext/models/asynchronous/RevAiJobOptions.java
Show resolved
Hide resolved
src/main/java/ai/rev/speechtotext/models/asynchronous/Summarization.java
Show resolved
Hide resolved
src/main/java/ai/rev/speechtotext/models/asynchronous/SummarizationFormattingOptions.java
Show resolved
Hide resolved
src/main/java/ai/rev/speechtotext/models/asynchronous/SummarizationJobStatus.java
Show resolved
Hide resolved
src/main/java/ai/rev/speechtotext/models/asynchronous/SummarizationOptions.java
Show resolved
Hide resolved
src/main/java/ai/rev/speechtotext/models/asynchronous/TranslationJobStatus.java
Show resolved
Hide resolved
src/main/java/ai/rev/speechtotext/models/asynchronous/TranslationLanguage.java
Show resolved
Hide resolved
3856694
to
c5f6812
Compare
|
||
import com.google.gson.annotations.SerializedName; | ||
|
||
/** Model type. */ |
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.
This looks like can be more verbose
|
||
@Override | ||
public String toString() { | ||
return "{" + "model='" + model + '\'' + '}'; |
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.
what is the purpose of having it in this format? why can't consumer of this type format it as needed?
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.
I don't know what the intention was, but I follow the same pattern. JobOptions class at the top level calls .toString() on every member and expects this output.
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.
yeah, really. at last for some nested models we have this formatting.
@amikofalvy by any chance do you know what was the purpose?
this.summarizationOptions = summarizationOptions; | ||
} | ||
|
||
public TranslationOptions getTranslationOptions() { |
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.
I think here should be a comment
private String prompt; | ||
|
||
/** Standard or Premium AI backend. * */ | ||
@SerializedName("model") |
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.
not sure we can have any other model type, but we can say something like summarization model
and then @see NlpModel
. then we can extend the NlpModel as we need without other changes
|
||
import com.google.gson.annotations.SerializedName; | ||
|
||
/** Summarization formatting options. * */ |
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.
is there a purpose for * */
? I see it everywhere in this PR
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.
This is how autoformatted puts it
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.
I did not find anything relevant in Google. Also, I don't see it like this in other files. Even in some of your files there is a single star only
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.
Okay. Thanks auto formatter! :-)
I'll spend some more time on this.
REVAI-3854: Initial draft