Added setEnableWordTimeOffsets(true) to Async Recognize for a File #781

dlaqab · 2017-08-01T21:41:53Z

Added setEnableWordTimeOffsets(true) to Async Recognize for a File
Added setEnableWordTimeOffsets(false) to Sync samples and quick start

Added setEnableWordTimeOffsets(false) to Sync samples and quick start

googlebot · 2017-08-01T21:41:55Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

dlaqab · 2017-08-01T21:42:25Z

I am covered by my current employer (Google)

gguuss · 2017-08-01T21:55:06Z

speech/cloud-client/src/main/java/com/example/speech/Recognize.java

@@ -127,6 +128,7 @@ public static void syncRecognizeGcs(String gcsUri) throws Exception, IOException
        .setEncoding(AudioEncoding.FLAC)
        .setLanguageCode("en-US")
        .setSampleRateHertz(16000)
+        .setEnableWordTimeOffsets(false)


Isn't false the default?

gguuss · 2017-08-01T21:55:18Z

speech/cloud-client/src/main/java/com/example/speech/Recognize.java

@@ -95,6 +95,7 @@ public static void syncRecognizeFile(String fileName) throws Exception, IOExcept
        .setEncoding(AudioEncoding.LINEAR16)
        .setLanguageCode("en-US")
        .setSampleRateHertz(16000)
+        .setEnableWordTimeOffsets(false)


Isn't false the default?

gguuss · 2017-08-01T21:58:52Z

speech/cloud-client/src/main/java/com/example/speech/Recognize.java

@@ -186,6 +189,11 @@ public static void asyncRecognizeFile(String fileName) throws Exception, IOExcep
      List<SpeechRecognitionAlternative> alternatives = result.getAlternativesList();
      for (SpeechRecognitionAlternative alternative: alternatives) {
        System.out.printf("Transcription: %s%n", alternative.getTranscript());
+        for (WordInfo wordInfo: alternative.getWordsList()) {


Please add a test for this.

Done. I added the test. I also fixed the test failure in AsyncRecognizeGcs test, so all the tests should pass now

gguuss · 2017-08-01T21:59:13Z

speech/cloud-client/src/main/java/com/example/speech/Recognize.java

+        for (WordInfo wordInfo: alternative.getWordsList()) {
+          System.out.println(wordInfo.getWord());
+          System.out.printf("\t%s ns - %s ns\n",
+              wordInfo.getStartTime().getNanos(), wordInfo.getEndTime().getNanos());


You're missing seconds, this just shows the nanoseconds for portions of the response.

Please update the other async sample to correctly show the seconds and nanos calculated to second fractions.

your original code did not have seconds, and I did not change the "pretty print" to show seconds.

gguuss · 2017-08-01T22:31:44Z

speech/cloud-client/src/main/java/com/example/speech/QuickstartSample.java

@@ -50,6 +50,7 @@ public static void main(String... args) throws Exception {
        .setEncoding(AudioEncoding.LINEAR16)
        .setSampleRateHertz(16000)
        .setLanguageCode("en-US")
+        .setEnableWordTimeOffsets(false)


Isn't this superfluous as false is the default?

I'm pretty sure we don't want to set an optional parameter to its default value in the quickstart example. Thoughts?

Fixed the tests for WordTimeOffsets

googlebot · 2017-08-02T16:55:03Z

CLAs look good, thanks!

gguuss · 2017-08-02T18:15:20Z

speech/cloud-client/src/main/java/com/example/speech/Recognize.java

@@ -185,7 +188,12 @@ public static void asyncRecognizeFile(String fileName) throws Exception, IOExcep
    for (SpeechRecognitionResult result: results) {
      List<SpeechRecognitionAlternative> alternatives = result.getAlternativesList();
      for (SpeechRecognitionAlternative alternative: alternatives) {
-        System.out.printf("Transcription: %s%n", alternative.getTranscript());
+        System.out.printf("Transcription: %s\n",alternative.getTranscript());


This should be:

System.out.printf("\t%s.%s sec - %s.%s sec\n", wordInfo.getStartTime().getSeconds(), wordInfo.getStartTime().getNanos() / 100000000, wordInfo.getEndTime().getSeconds(), wordInfo.getEndTime().getNanos() / 100000000); }

This line shows the all up transcript - lines bellow it are iterating through the words and displaying the start and end time stamps

gguuss · 2017-08-02T18:16:08Z

speech/cloud-client/src/test/java/com/example/speech/RecognizeIT.java

    Recognize.asyncRecognizeGcs(gcsPath);
    String got = bout.toString();
-    assertThat(got).contains("\t0.0 sec -");
+    assertThat(got).contains("\t0 ns");


Revert this change when you correct the print output in asyncRecognizeFile

gguuss · 2017-08-02T18:16:57Z

@dlaqab Please update your branch to match master, change your new code to match the updated start seconds / end seconds code as:

          System.out.printf("\t%s.%s sec - %s.%s sec\n",
              wordInfo.getStartTime().getSeconds(),
              wordInfo.getStartTime().getNanos() / 100000000,
              wordInfo.getEndTime().getSeconds(),
              wordInfo.getEndTime().getNanos() / 100000000);
        }

Revert your change in RecognizeIT to test for actual seconds / fractions of a second.

gguuss · 2017-08-02T23:45:12Z

@lesv /FYI - Looks like the Circle tests are not running because this is on a personal fork of the repo. When I run the tests locally, the following is my output:

[INFO] Scanning for projects...
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building speech-google-cloud-samples 1.0.5
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- jacoco-maven-plugin:0.7.9:prepare-agent (default) @ speech-google-cloud-samples ---
[INFO] argLine set to -javaagent:/Users/class/.m2/repository/org/jacoco/org.jacoco.agent/0.7.9/org.jacoco.agent-0.7.9-runtime.jar=destfile=/Users/class/temp/daryush/java-docs-samples/speech/cloud-client/target/jacoco.exec
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ speech-google-cloud-samples ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/src/main/resources
[INFO]
[INFO] --- maven-compiler-plugin:3.6.1:compile (default-compile) @ speech-google-cloud-samples ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ speech-google-cloud-samples ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/src/test/resources
[INFO]
[INFO] --- maven-compiler-plugin:3.6.1:testCompile (default-testCompile) @ speech-google-cloud-samples ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-surefire-plugin:2.20:test (default-test) @ speech-google-cloud-samples ---
[INFO]
[INFO] --- jacoco-maven-plugin:0.7.9:report (report) @ speech-google-cloud-samples ---
[INFO] Loading execution data file /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/target/jacoco.exec
[INFO] Analyzed bundle 'speech-google-cloud-samples' with 3 classes
[INFO]
[INFO] --- maven-jar-plugin:2.4:jar (default-jar) @ speech-google-cloud-samples ---
[INFO]
[INFO] --- maven-failsafe-plugin:2.20:integration-test (default) @ speech-google-cloud-samples ---
[INFO]
[INFO] -------------------------------------------------------
[INFO] T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.example.speech.QuickstartSampleIT
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.831 s - in com.example.speech.QuickstartSampleIT
[INFO] Running com.example.speech.RecognizeIT
[INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 52.388 s - in com.example.speech.RecognizeIT
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 58.288 s
[INFO] Finished at: 2017-08-02T16:42:02-07:00
[INFO] Final Memory: 21M/376M
[INFO] ------------------------------------------------------------------------

Checkstyle shows:

[INFO] Scanning for projects...
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building speech-google-cloud-samples 1.0.5
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-checkstyle-plugin:2.17:check (default-cli) @ speech-google-cloud-samples ---
[INFO] Starting audit...
[WARN] /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/src/main/java/com/example/speech/QuickstartSample.java:36:3: Missing a Javadoc comment. [JavadocMethod]
[WARN] /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/src/main/java/com/example/speech/Recognize.java:47:3: Missing a Javadoc comment. [JavadocMethod]
[WARN] /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/src/main/java/com/example/speech/Recognize.java:157:3: Missing a Javadoc comment. [JavadocMethod]
[WARN] /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/src/test/java/com/example/speech/QuickstartSampleIT.java:35: Abbreviation in name 'QuickstartSampleIT' must contain no more than '2' consecutive capital letters. [AbbreviationAsWordInName]
[WARN] /Users/class/temp/daryush/java-docs-samples/speech/cloud-client/src/test/java/com/example/speech/RecognizeIT.java:35: Abbreviation in name 'RecognizeIT' must contain no more than '2' consecutive capital letters. [AbbreviationAsWordInName]
Audit done.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.491 s
[INFO] Finished at: 2017-08-02T16:44:34-07:00
[INFO] Final Memory: 18M/308M
[INFO] ------------------------------------------------------------------------

So mostly LGTM, I'm just a little concerned about setting the optional parameter to its default value in the Quickstart still.

lesv · 2017-08-03T00:50:47Z

@gguuss I'll try to fix that tomorrow.

gguuss

I feel we may want to at some point remove the .setEnableWordTimeOffsets(false) calls but reluctantly approving for today.

googlebot · 2017-08-03T19:37:18Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

gguuss · 2017-08-03T19:38:48Z

I signed it!

gguuss

Will make change separately to match other samples.

beccasaurus · 2017-08-04T03:03:03Z

This can be closed.

dlaqab · 2017-08-04T03:17:57Z

why? I am not sure why this needs to be closed

beccasaurus · 2017-08-04T04:00:57Z

The default for EnableWordTimeOffsets is false; it is incorrect to add it explicitly.
None of the other languages do this, making Java inconsistent.

beccasaurus · 2017-08-04T06:06:42Z

The setEnableWordTimeOffsets() call in these samples is extraneous to the task these samples are intended to demonstrate. Closing.

beccasaurus · 2017-08-04T06:08:13Z

Note: samples which demonstrate Enable Word Time Offsets were added to java-docs-samples in #787

Added setEnableWordTimeOffsets(true) to Async Recognize for a File

1e4cf44

Added setEnableWordTimeOffsets(false) to Sync samples and quick start

googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Aug 1, 2017

gguuss suggested changes Aug 1, 2017

View reviewed changes

Added missing test for AsyncRecognizeFile

a893740

Fixed the tests for WordTimeOffsets

googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Aug 2, 2017

gguuss reviewed Aug 2, 2017

View reviewed changes

Daryush Laqab added 2 commits August 2, 2017 11:17

Merge remote-tracking branch 'upstream/master'

29f9f76

Changed Nanoseconds to Seconds, and fixed the tests

e4a7cce

gguuss approved these changes Aug 3, 2017

View reviewed changes

Merge branch 'master' into dalaqab_enableWordTimeOffsets_examples

3384179

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Aug 3, 2017

gguuss suggested changes Aug 3, 2017

View reviewed changes

beccasaurus self-requested a review August 4, 2017 04:01

beccasaurus closed this Aug 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added setEnableWordTimeOffsets(true) to Async Recognize for a File #781

Added setEnableWordTimeOffsets(true) to Async Recognize for a File #781

dlaqab commented Aug 1, 2017

googlebot commented Aug 1, 2017

dlaqab commented Aug 1, 2017

gguuss Aug 1, 2017

gguuss Aug 1, 2017

gguuss Aug 1, 2017

dlaqab Aug 2, 2017

gguuss Aug 1, 2017

gguuss Aug 1, 2017

dlaqab Aug 2, 2017

gguuss Aug 1, 2017

gguuss Aug 2, 2017

googlebot commented Aug 2, 2017

gguuss Aug 2, 2017

dlaqab Aug 2, 2017

gguuss Aug 2, 2017

gguuss commented Aug 2, 2017

gguuss commented Aug 2, 2017 •

edited

Loading

lesv commented Aug 3, 2017

gguuss left a comment

googlebot commented Aug 3, 2017

gguuss commented Aug 3, 2017

gguuss left a comment

beccasaurus commented Aug 4, 2017

dlaqab commented Aug 4, 2017

beccasaurus commented Aug 4, 2017

beccasaurus commented Aug 4, 2017

beccasaurus commented Aug 4, 2017

Added setEnableWordTimeOffsets(true) to Async Recognize for a File #781

Added setEnableWordTimeOffsets(true) to Async Recognize for a File #781

Conversation

dlaqab commented Aug 1, 2017

googlebot commented Aug 1, 2017

dlaqab commented Aug 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

googlebot commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gguuss commented Aug 2, 2017

gguuss commented Aug 2, 2017 • edited Loading

lesv commented Aug 3, 2017

gguuss left a comment

Choose a reason for hiding this comment

googlebot commented Aug 3, 2017

gguuss commented Aug 3, 2017

gguuss left a comment

Choose a reason for hiding this comment

beccasaurus commented Aug 4, 2017

dlaqab commented Aug 4, 2017

beccasaurus commented Aug 4, 2017

beccasaurus commented Aug 4, 2017

beccasaurus commented Aug 4, 2017

gguuss commented Aug 2, 2017 •

edited

Loading