Skip to content

Translation V3 GA samples #2478

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 18 commits into from
Oct 18, 2019
Merged

Translation V3 GA samples #2478

merged 18 commits into from
Oct 18, 2019

Conversation

@munkhuushmgl
Copy link
Contributor

@munkhuushmgl munkhuushmgl commented Oct 15, 2019 •
edited by sirtorry
Loading

need manual samples:
batch_translate_text_with_glossary_and_model
batch_translate_text_with_glossary

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 15, 2019
@sirtorry sirtorry requested review from nnegrey and sirtorry October 15, 2019 22:05
@sirtorry sirtorry added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 16, 2019
sirtorry
sirtorry suggested changes Oct 16, 2019
View reviewed changes
Copy link
Contributor

@sirtorry sirtorry left a comment

Choose a reason for hiding this comment

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

  1. we should consider adding label samples or adding label arguments to translate, batch translate and detect language snippets
  2. we should consider adding a detect_language_with_model sample
  3. we should consider adding a get_supported_languages_for_model sample

@munkhuushmgl munkhuushmgl requested a review from sirtorry October 16, 2019 02:33
@nnegrey
Copy link
Contributor

nnegrey commented Oct 16, 2019

Quick question, what's the the v2 samples?
Shouldn't those already exist in some files and be left as is for the docs to stay the same?

nnegrey
nnegrey reviewed Oct 16, 2019
View reviewed changes
nnegrey
nnegrey reviewed Oct 16, 2019
View reviewed changes
nnegrey
nnegrey reviewed Oct 16, 2019
View reviewed changes
@nnegrey
Copy link
Contributor

nnegrey commented Oct 16, 2019

@sirtorry

  1. we should consider adding label samples or adding label arguments to translate, batch translate and detect language snippets
  2. we should consider adding a detect_language_with_model sample
  3. we should consider adding a get_supported_languages_for_model sample

For 2 and 3. I don't think the product team asked for them or the docs either, so if we don't have them in the docs, I don't think we need them.

nnegrey
nnegrey reviewed Oct 16, 2019
View reviewed changes
@sirtorry
Copy link
Contributor

sirtorry commented Oct 16, 2019

Quick question, what's the the v2 samples?
Shouldn't those already exist in some files and be left as is for the docs to stay the same?

yes, but as it stands right now, the region tag and file names are really confusing. i believe it is important to clarify which examples are for v2 and which are for v3.

@munkhuushmgl munkhuushmgl requested a review from nnegrey October 16, 2019 21:25
@sirtorry
Copy link
Contributor

sirtorry commented Oct 16, 2019

After discussing with @nnegrey, let's remove v2 samples from this PR.

@nnegrey nnegrey added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 17, 2019
@munkhuushmgl munkhuushmgl requested a review from nnegrey October 18, 2019 00:13
@nnegrey
Copy link
Contributor

nnegrey commented Oct 18, 2019

I just pulled the latest and a lot of the tests are failing for me.

@nnegrey
Copy link
Contributor

nnegrey commented Oct 18, 2019

LGTM
@sirtorry

sirtorry
sirtorry approved these changes Oct 18, 2019
View reviewed changes
Copy link
Contributor

@sirtorry sirtorry left a comment

Choose a reason for hiding this comment

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

tests pass locally on noah's machine. lgtm.

@nnegrey nnegrey merged commit 824d83b into GoogleCloudPlatform:tmp-generated-samples Oct 18, 2019
@munkhuushmgl munkhuushmgl mentioned this pull request Oct 28, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@nnegrey nnegrey nnegrey approved these changes

@sirtorry sirtorry sirtorry approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement. kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@munkhuushmgl @nnegrey @sirtorry @googlebot