Skip to content

gh-114258: Use Argument Clinic for __len__ - #160012

Open
corona10 wants to merge 6 commits into
python:mainfrom
corona10:gh-114258-len
Open

corona10 wants to merge 6 commits into
python:mainfrom
corona10:gh-114258-len

Conversation

@corona10

@corona10 corona10 commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

@corona10

Copy link
Copy Markdown
Member Author

cc @ZeroIntensity

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One formatting nitpick

Comment thread Tools/clinic/libclinic/slots.py Outdated
Comment on lines +51 to +54
SlotSpec('__next__', 'tp_iternext', 'PyObject *', 'object'),
),
'__len__': (
SlotSpec('__len__', 'sq_length', 'Py_ssize_t', 'Py_ssize_t'),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you use keyword arguments instead of positional args here? I think it would make this more readable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure looks better

Comment thread Tools/clinic/libclinic/slots.py Outdated
'__next__': (
SlotSpec('__next__', 'tp_iternext', 'PyObject *', 'object'),
),
'__len__': (

@corona10 corona10 Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For the __getitem__, it would be like this:

'__getitem__': (
    # sq_item
    SlotSpec( return_converter='object', parameters=(Py_ssize_t_converter,)),
    # mp_subscript
    SlotSpec(return_converter='object', parameters=(object_converter,)),
),

Comment thread Tools/clinic/libclinic/slots.py Outdated
'__next__': SlotSpec('__next__', 'PyObject *', 'object', {
(): 'tp_iternext',
}),
'__next__': SlotSpec(return_type='PyObject *'),

@corona10 corona10 Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ZeroIntensity

Can you please take a look again? I simplify everything as possible.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Wait..

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah PTAL with the latest version.
I try to maintain consistency with other AC implementations as much as possible, while removing unnecessary guardrails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants