-
Notifications
You must be signed in to change notification settings - Fork 265
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
~~[Widgets] Don't call api inference if output exists~~ #1017
Conversation
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 this looks good to me, though the code (not specific to this PR) looks a bit hard to reason about for me...
|
||
async function getOutput({ withModelLoading = false, isOnLoadCall = false, sampleOutput = undefined }: GetOutputOptions = {}) { | ||
// don't call api-inference if sample output already exists | ||
if(isValidOutputLabels(sampleOutput)) { |
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.
(code needs formatting)
...omponents/InferenceWidget/widgets/AudioClassificationWidget/AudioClassificationWidget.svelte
Outdated
Show resolved
Hide resolved
Logic sounds good to me too |
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.
Nice!
...omponents/InferenceWidget/widgets/AudioClassificationWidget/AudioClassificationWidget.svelte
Outdated
Show resolved
Hide resolved
6c8db96
to
2cc7ec1
Compare
2cc7ec1
to
cd15b19
Compare
Superceded by #1063 |
Superceded by #1063
Closes #1011
Follow up to #979
#979 correctly implemented behaviour for the previewInputSample function here. Which is: don't call api-inference if
sample.output
existsThe same should be the case for
applyInputSample
andonMount
example (which is this PR). Don't call api-inference ifsample.output
exists. #1011 happened because api-inference was called and there was no cached example althoughsample.output
was in the repoWdyt of the proposed solution? I've only implemented for AudioClassificationWidget.svelte. If others agree with the design, will implement for other widgets as well
** for the clarification,
previewInputSample
function gets called when you hover over different input samples.applyInputSample
function gets called when you end up clicking/selecting inout sampleOrder of merge operations