Skip to content
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

[speech-commands] Add support for custom model and metadata URLs #85

Merged
merged 9 commits into from Oct 12, 2018

Conversation

caisq
Copy link
Collaborator

@caisq caisq commented Oct 11, 2018

With unit tests.


This change is Reviewable

@caisq caisq requested a review from bileschi October 11, 2018 19:06
@caisq
Copy link
Collaborator Author

caisq commented Oct 11, 2018

cc @pyu10055

Copy link
Contributor

@bileschi bileschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 6 of 9 files at r1, 1 of 3 files at r2.
Reviewable status: 0 of 1 approvals obtained (waiting on @caisq and @bileschi)


speech-commands/src/browser_fft_recognizer.ts, line 71 at r2 (raw file):

   * @param modelURL An optional, custom model URL pointing to a model.json
   *   file. Supported schemes: http://, https://, and node.js-only: file://.
   *   Mutually exclusive with `vocabulary`. If provided, `customMetadatURL`

i think you meant metadataURL here


speech-commands/src/browser_fft_recognizer.ts, line 74 at r2 (raw file):

   *   most also be provided.
   * @param metadataURL A custom metadata URL pointing to a metadata.json
   *   file. Must be provided together with `customModelURL`.

typo old argument name


speech-commands/src/browser_fft_utils.ts, line 29 at r2 (raw file):

··

lint error maybe? spaces at end of line?


speech-commands/src/browser_fft_utils.ts, line 34 at r2 (raw file):

    return content;
  } else {
    throw new Error(`Unsupported URL scheme in metadata URL: ${url}`);

Perhaps add here that URL must begin with one of 'http://' 'https://' or 'file://' (nodeJS only)

Copy link
Collaborator Author

@caisq caisq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @bileschi)


speech-commands/src/browser_fft_recognizer.ts, line 71 at r2 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…

i think you meant metadataURL here

Done.


speech-commands/src/browser_fft_recognizer.ts, line 74 at r2 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…

typo old argument name

Done.


speech-commands/src/browser_fft_utils.ts, line 29 at r2 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…
··

lint error maybe? spaces at end of line?

Done.


speech-commands/src/browser_fft_utils.ts, line 34 at r2 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…

Perhaps add here that URL must begin with one of 'http://' 'https://' or 'file://' (nodeJS only)

Done.

@caisq caisq merged commit 6a84944 into tensorflow:master Oct 12, 2018
@caisq caisq deleted the speech-commands-custom-urls branch October 12, 2018 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants