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

Add count_text_tokens, and expose operations. #76

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

MarkDaoust
Copy link
Collaborator

Description of the change

  • Add count_text_tokens
  • Expose the operations service in the public api.

Motivation

These should have been in v1beta2.

Type of change

Feature

Checklist

  • I have performed a self-review of my code.
  • I have added detailed comments to my code where applicable.
  • I have verified that my change does not break existing code.
  • My PR is based on the latest changes of the main branch (if unsure, please run git pull --rebase upstream main).
  • I am familiar with the Google Style Guide for the language I have coded in.
  • I have read through the Contributing Guide and signed the Contributor License Agreement.

@MarkDaoust MarkDaoust requested a review from a team as a code owner October 4, 2023 20:23
@github-actions github-actions bot added the status:awaiting review PR awaiting review from a maintainer label Oct 4, 2023
markmcd
markmcd previously approved these changes Oct 5, 2023
@@ -114,6 +114,31 @@ def get_tuned_model(
return model_types.decode_tuned_model(result)


def get_base_model_name(
model: model_types.AnyModelNameOptions, client: glm.ModelServiceClient | None = None
):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
):
) -> str:

@MarkDaoust
Copy link
Collaborator Author

Merge #78 first.

markmcd
markmcd previously approved these changes Oct 10, 2023
Copy link
Member

@markmcd markmcd left a comment

Choose a reason for hiding this comment

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

Yay text counting :)

google/generativeai/text.py Outdated Show resolved Hide resolved
google/generativeai/text.py Show resolved Hide resolved

response = text_service.count_text_tokens(model, "Tell me a story about a magic backpack.")
self.assertEqual({"token_count": 7}, response)
if len(self.observed_requests) > 1:
Copy link
Member

Choose a reason for hiding this comment

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

Can we make the guard condition here something that makes it clear why it's optional? e.g. if make_model_name(model).startswith("tunedModel/"): ... or an extra param like should_look_up_model=bool

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@MarkDaoust MarkDaoust merged commit 125c436 into google-gemini:main Oct 11, 2023
6 checks passed
@github-actions github-actions bot removed the status:awaiting review PR awaiting review from a maintainer label Oct 11, 2023
markmcd pushed a commit to markmcd/generative-ai-python that referenced this pull request Oct 30, 2023
* Add count_text_tokens, and expose operations.

* Format and fix pytype errors.

* use get_base_model_name in create_tuned_model

* Resolve comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:python sdk Issue/PR related to Python SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants