Skip to content

Conversation

MarkDaoust
Copy link
Collaborator

@MarkDaoust MarkDaoust commented May 14, 2024

Fixes: #326

Change-Id: Ic20e2f88427d2e4fbc97847cf5c2df1f80a9a5a1
@github-actions github-actions bot added status:awaiting review PR awaiting review from a maintainer component:python sdk Issue/PR related to Python SDK labels May 14, 2024
@MarkDaoust MarkDaoust marked this pull request as draft May 14, 2024 21:21
@mayureshagashe2105
Copy link
Contributor

mayureshagashe2105 commented May 15, 2024

model = genai.GenerativeModel(model_name, system_instruction="Talk in rhymes")
model.count_tokens("")

I think this would still raise the following :

https://github.com/google-gemini/generative-ai-python/blob/efead6bea6768f6f4a3d90d348647b0a54fe2435/google/generativeai/types/content_types.py#L267-L269

@MarkDaoust MarkDaoust changed the title Allow empty contents with count_tokens Allow count_tokens with no contents. May 20, 2024
@MarkDaoust
Copy link
Collaborator Author

Hmmm... It goes through to_contentS first, that catches None:

https://github.com/google-gemini/generative-ai-python/blob/efead6bea6768f6f4a3d90d348647b0a54fe2435/google/generativeai/types/content_types.py#L300-L303

But to should it let "" through?

Updated title.

@MarkDaoust MarkDaoust marked this pull request as ready for review May 20, 2024 22:07
@MarkDaoust MarkDaoust merged commit 05877f7 into google-gemini:main May 21, 2024
@github-actions github-actions bot removed the status:awaiting review PR awaiting review from a maintainer label May 21, 2024
@mayureshagashe2105
Copy link
Contributor

mayureshagashe2105 commented May 21, 2024

It goes through to_contentS first, that catches None

I don't think it will(tested it locally):
"" is not None.
It will go to to_content method and would raise the error. Even if we change to_contents to handle "", something like:

 def to_contents(contents: ContentsType) -> list[glm.Content]: 
     if not contents: 
         return [] 

The request goes through but the API raises error as contents is a required field for the GenerateContentRequest

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.

count_tokens should allow empty content

3 participants