Skip to content

Add GCS examples for Natural Language #432

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Dec 5, 2016
Merged

Add GCS examples for Natural Language #432

merged 7 commits into from
Dec 5, 2016

Conversation

gguuss
Copy link
Contributor

@gguuss gguuss commented Dec 2, 2016

Adds Google Cloud Storage (GCS) examples and tests to be at parity with other samples for inclusion in snippets.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 2, 2016
jerjou
jerjou suggested changes Dec 2, 2016
View reviewed changes
language/analysis/README.md
@@ -30,12 +30,13 @@ mvn clean compile assembly:single
```

We can then run the assembled JAR file with the `java` command. The variable $COMMAND takes
three values `entities`, `sentiment` or `syntax`.
six values `entities-text`, `entities-file`, `sentiment-text`, `sentiment-file`,
`syntax-text`, or `syntax-file`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Oi. Perhaps would be cleaner to just detect a gcs uri? As in:

if (command.equals("entities")) {
  if (text.startsWith("gs://")) {
    app.analyzeEntitiesFile(text);
  } else {
    app.analyzeEntitiesText(text);
  }
} // ... etc

inside Analyze.java

Copy link
Member

Choose a reason for hiding this comment

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

That would moves us toward writing library code in the snippets, and the other 6 languages have separate snippets for raw strings vs gcs files, so Java would need to do the same to fit into the docs where the other snippets live.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think he just means in the command parser, I am in favor of simplifying the args parser for a lighter-weight usage string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup - yay!

language/analysis/src/main/java/com/google/cloud/language/samples/Analyze.java
@@ -164,19 +170,42 @@ public Analyze(LanguageServiceClient languageApi) {
}

/**
* Gets {@link Entity}s from the string representing the GCS {@code path}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps:

* Gets {@link Entity}s from the contents of the object at the given GCS {@code path}

Otherwise it sounds like it's looking for entities in the path string itself. (here and below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

language/analysis/src/test/java/com/google/cloud/language/samples/AnalyzeIT.java

// Assert
assertThat((double)sentiment.getMagnitude()).isGreaterThan(1.0);
assertThat((double)sentiment.getScore()).isWithin(0.0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a bit brittle, no? Perhaps isWithin(0.1)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, changed to 0.1

jerjou
jerjou approved these changes Dec 2, 2016
View reviewed changes
Copy link
Contributor

@jerjou jerjou left a comment

Choose a reason for hiding this comment

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

@lesv FYI

@gguuss gguuss merged commit 7770807 into master Dec 5, 2016
@gguuss gguuss deleted the language-gcs branch December 5, 2016 21:32
lesv
lesv reviewed Dec 5, 2016
View reviewed changes
Copy link
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

LGTM - though shouldn't we have used "${QUOTE}" https://google.github.io/styleguide/shell.xml#Use_Local_Variables ?? (in your README.md)

@gguuss
Copy link
Contributor Author

gguuss commented Dec 5, 2016

Sorry for merging a little aggressively and good catch - I'll fix the style for shell variables in a separate PR if that works. Should we also be making $JAR_FILE and $MAIN_CLASS local variables as well?

@lesv
Copy link
Contributor

lesv commented Dec 5, 2016

I think that's more inline with the style guide. (ie. YES)

@gguuss
Copy link
Contributor Author

gguuss commented Dec 5, 2016

SGTM.

Shabirmean pushed a commit that referenced this pull request Nov 14, 2022
@renovate-bot
test(deps): update dependency junit:junit to v4.13.2 (#432)
0f37854 
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [junit:junit](http://junit.org) ([source](https://togithub.com/junit-team/junit4)) | `4.13.1` -> `4.13.2` | [![age](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/compatibility-slim/4.13.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/confidence-slim/4.13.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Renovate configuration

:date: **Schedule**: At any time (no schedule defined).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-dialogflow).
@Shabirmean Shabirmean mentioned this pull request Nov 15, 2022
Merged
9 tasks
Shabirmean pushed a commit that referenced this pull request Nov 15, 2022
@renovate-bot @Shabirmean
test(deps): update dependency junit:junit to v4.13.2 (#432)
30236c8 
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [junit:junit](http://junit.org) ([source](https://togithub.com/junit-team/junit4)) | `4.13.1` -> `4.13.2` | [![age](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/compatibility-slim/4.13.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/junit:junit/4.13.2/confidence-slim/4.13.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Renovate configuration

:date: **Schedule**: At any time (no schedule defined).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-dialogflow).
Sita04 pushed a commit that referenced this pull request Feb 7, 2023
@release-please
chore(master): release 2.2.8-SNAPSHOT (#432)
e564a0d 
🤖 I have created a release \*beep\* \*boop\* 
---
### Updating meta-information for bleeding-edge SNAPSHOT release.
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesv lesv lesv left review comments

@jerjou jerjou jerjou approved these changes

@jmdobry jmdobry jmdobry approved these changes

Assignees

@jerjou jerjou

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gguuss @lesv @jerjou @jmdobry @googlebot