Skip to content

Conversation

@peter-soos
Copy link
Contributor

@peter-soos peter-soos commented Nov 19, 2025

fixes #8791

@github-actions github-actions bot added c++ codegen Involving generating code from schema python labels Nov 19, 2025
@peter-soos peter-soos changed the title [Python] Fix inconsistent creator function naming in generated code (#8791) [Python] Fix inconsistent creator function naming in generated code #8791 Nov 19, 2025
@peter-soos peter-soos changed the title [Python] Fix inconsistent creator function naming in generated code #8791 [Python] Fix inconsistent creator function naming in generated code Nov 19, 2025
@jtdavis777 jtdavis777 self-requested a review November 24, 2025 12:15
@jtdavis777
Copy link
Collaborator

this PR will likely need to regenerate a bunch of the generated code to pass

@jtdavis777
Copy link
Collaborator

jtdavis777 commented Nov 26, 2025

I stand corrected. pulled your branch and verified this is good. sorta surprised none of the generated code hit this path. I'll test this manually on some of my fbs files and then we can ship it!

@jtdavis777
Copy link
Collaborator

before I merge this, @fliiiix would you give this a quick gut check? I can't tell which is better, to change the Create* function name, or to fix the pack function. My gut tells me that fixing pack would be less disruptive, if possibly less correct.

@fliiiix
Copy link
Contributor

fliiiix commented Nov 27, 2025

A few notes:

The pyi files should stay in sync, so this would also need to be changed:

    stub << "def Create" + namer_.Type(*struct_def)
         << "(builder: flatbuffers.Builder";

A test would be good, it a bit of a red flag that it doesn't show up in any generated files :D

I can't tell which is better

Based on my understanding as long as they match its fine but if we look at namer.h i would say the pack function is more correct and it should be namer_.Function on both ends.

@peter-soos thanks for the great issue with a easy to reproduce case ✨

@jtdavis777
Copy link
Collaborator

I agree that namer_.Function is more "right" - just worried that the free function is more used (and visible) -- merging this change may break more peoples' code than keeping the existing wrong name.

@fliiiix
Copy link
Contributor

fliiiix commented Nov 27, 2025

just worried that the free function is more used

good point, i personally would risk it based on the assumption that not a lot of people use structs and even less use _ in the name (because for most names Type and Function are equal)

@jtdavis777 jtdavis777 merged commit 4b823b1 into google:master Nov 29, 2025
50 checks passed
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.

[Python] Inconsistent creator function naming in generated code

3 participants