Skip to content

fix(emitter): pass group_id by keyword to avoid str(None) on clone - #1598

Closed
re2zero wants to merge 1 commit into
i-am-bee:mainfrom
re2zero:main
Closed

re2zero wants to merge 1 commit into
i-am-bee:mainfrom
re2zero:main

Conversation

@re2zero

@re2zero re2zero commented Aug 15, 2026

Copy link
Copy Markdown

When group_id is None (the default), str(None) produces the literal string 'None' on clone, corrupting every event emitted by the cloned emitter and its descendants.

Changes

  • Pass constructor arguments by keyword in Emitter.clone() instead of positionally, so group_id=None stays None.
  • Added test test_clone_without_group_id that exercises the default-None path, which the existing test (always uses group_id="test_group") never covered.

Fixes #1581

When group_id is None, str(None) produces the literal string 'None',
corrupting every event emitted by the cloned emitter and its descendants.

Pass group_id by keyword argument instead, so None stays None.
Fixes i-am-bee#1581
@re2zero
re2zero requested a review from a team as a code owner August 15, 2026 18:18
@github-actions github-actions Bot added the python Python related functionality label Aug 15, 2026
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Aug 15, 2026
@Tomas2D

Tomas2D commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Thanks @re2zero! This is a correct fix for #1581. Two things to note:

  1. DCO is failing — please sign off your commits (git commit --amend -s / git rebase --signoff then force-push).
  2. creator=self.creator if self.creator else None is slightly lossy — it would turn a falsy-but-valid creator into None. creator=self.creator is more faithful.
  3. Heads-up: the same emitter change is also included in your chore: allow Python 3.14 by relaxing version constraint #1599 (which is otherwise about Python 3.14). The two PRs overlap — it's cleaner to keep this emitter fix here and remove it from chore: allow Python 3.14 by relaxing version constraint #1599 (see my note there).

We currently have three PRs for #1581; we're leaning toward #1582 (from the issue reporter) as it's already fully reviewed. Thanks for the contribution regardless! 🐝

@Tomas2D

Tomas2D commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

#1581 has now been fixed and merged via #1582, so I'm closing this one as resolved. Thanks for the fix, @re2zero! Note your #1599 is still open — see my note there; its emitter changes are now redundant, so that PR should be rescoped to just the Python 3.14 work. 🐝

@Tomas2D Tomas2D closed this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Python related functionality size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Emitter.clone() turns an unset group_id into the literal string "None" (Python)

2 participants