Skip to content

Conversation

sethraymond
Copy link
Contributor

Type-hints for the Python Object API are incorrect for strings - the _UnPack() method does not convert bytes to strings. This PR introduces an optional flag (false by default) that enables decoding the bytes into string objects.

Resolves #5997

@github-actions github-actions bot added c++ codegen Involving generating code from schema python labels Mar 5, 2025
@sethraymond
Copy link
Contributor Author

Bump

@sethraymond
Copy link
Contributor Author

@aardappel bump, can we get the CI run on this PR?

@aardappel
Copy link
Collaborator

You can get the CI to run by rebasing, it is out of date.. I guess I'll do that for you.

return
self.name = nestedUnionTest.Name()
if self.name is not None:
self.name = self.name.decode('utf-8')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this meant to be self.name = nestedUnionTest.name.decode('utf-8') ?

Copy link
Contributor Author

@sethraymond sethraymond Apr 23, 2025

Choose a reason for hiding this comment

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

nestedUnionTest is not an object API type, so the only way to access the name from that object is via nestedUnionTest.Name().

This change is intentional, but if you're asking for a modification to the auto-generated code, I could change the autogeneration here to be

code += GetIndents(3) + "self." + field_field + " = " + struct_var + "." + field_method + "().decode('utf-8')";

Then this would read

self.name = nestedUnionTest.Name()
if self.name is not None:
    self.name = nestedUnionTest.Name().decode('utf-8')

It's functionally the same, though, since self.name is set to nestedUnionTest.name() a couple lines beforehand. I personally think the current implementation is a bit more readable, but I'm open to suggestions.

@aardappel
Copy link
Collaborator

Looks ok, I guess? @dbaileychess

@sethraymond
Copy link
Contributor Author

Bump @aardappel and @dbaileychess

@sethraymond
Copy link
Contributor Author

@aardappel, I see there's been some movement with other PRs - can this get merged? Thanks!

@aardappel
Copy link
Collaborator

Ok, lets merge, thanks!

@aardappel aardappel merged commit 7555643 into google:master Jun 29, 2025
47 of 50 checks passed
@fliiiix fliiiix mentioned this pull request Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ codegen Involving generating code from schema python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Object api strings return bytes instead of str [Python]

2 participants