Skip to content

Conversation

@AA-Turner
Copy link
Member

@AA-TurnerAA-Turner commented Aug 12, 2025

serhiy-storchaka

This comment was marked as resolved.

@ericsnowcurrently
Copy link
Member

Just be sure I understood, the problem is that the arg spec for _queues.put() has one too many items in it. Is that right?

I'm pretty sure the $p shouldn't be there. The "fallback" arg is an int flag 1, so it should have an "i" in the spec. Thus, the spec for queuesmod_create() should not change and for queuesmod_put() it should be "O&O|ii:put".

FWIW, at the time, with the feature freeze at hand, I didn't worry about ironing out little API quirks (e.g. placeholder args) in the low-level module. It isn't exposed to users, right?

Footnotes

  1. See resolve_fallback() in Modules/_interpreters_common.h.

@ericsnowcurrently
Copy link
Member

To be clear, I left room for other "fallback" options later, rather than it being a definite binary choice. Sorry that wasn't more clear. Then again, that decision was probably a premature accommodation for a possible future. I figure it doesn't matter much though.

@AA-Turner

This comment was marked as resolved.

@AA-Turner
Copy link
MemberAuthor

Just be sure I understood, the problem is that the arg spec for _queues.put() has one too many items in it. Is that right?

Yes, specifically that the format spec for PyArg_ParseTupleAndKeywords expected five args, but kwlist gave only four.

I'm pretty sure the $p shouldn't be there. The "fallback" arg is an int flag 1, so it should have an "i" in the spec. Thus, the spec for queuesmod_create() should not change and for queuesmod_put() it should be "O&O|ii:put".

I've updated the PR. I don't think we need the test anymore given this is the current behaviour -- we are just removing the errant $p. Please would you be able to review/approve the updated PR?

FWIW, at the time, with the feature freeze at hand, I didn't worry about ironing out little API quirks (e.g. placeholder args) in the low-level module. It isn't exposed to users, right?

Definitely the correct choice. _interpqueues is technically exposed to users by dint of being importable, but is entirely undocumented and of course a private module. This PR is just a correctness fix that Serhiy spotted, but it would nevertheless be good to include it in 3.14.0 candidate 3 / final.

A

Copy link
Member

@ericsnowcurrentlyericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@AA-TurnerAA-Turner merged commit 79aeeb8 into python:mainAug 21, 2025
46 checks passed
@miss-islington-app
Copy link

Thanks @AA-Turner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@AA-TurnerAA-Turner deleted the clinic/interpqueues-put branch August 21, 2025 22:01
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 21, 2025
…thonGH-137686) (cherry picked from commit 79aeeb8) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@bedevere-app
Copy link

GH-138034 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14 bugs and security fixes label Aug 21, 2025
@serhiy-storchaka
Copy link
Member

And I think that it would be better to remove passing explicit -1 as the fallback argument to _queue.create().

hugovk pushed a commit that referenced this pull request Aug 27, 2025
…H-137686) (#138034) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
kumaraditya303 pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2025
…)`` (pythonGH-137686) (python#138034) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AA-Turner@ericsnowcurrently@serhiy-storchaka