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.

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

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

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 sirtorry and nnegrey 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
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
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.

@sirtorry
Copy link
Contributor

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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants