Skip to content

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 7, 2025

@vstinner
Copy link
Member Author

vstinner commented Oct 7, 2025

cc @serhiy-storchaka @encukou

@vstinner
Copy link
Member Author

vstinner commented Oct 7, 2025

@serhiy-storchaka: I addressed your comments. Please review the updated PR.


tup = ()
copy = tuple_fromarray(tup)
self.assertIs(copy, tup)
Copy link
Member

Choose a reason for hiding this comment

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

Is not the empty tuple singleton a CPython implementation detail?

Copy link
Member Author

Choose a reason for hiding this comment

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

The whole file is a CPython implementation detail (_testcapi), no?

Use also UNINITIALIZED_SIZE in test code.
@vstinner
Copy link
Member Author

vstinner commented Oct 7, 2025

I updated the PR for the latest review.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

Comment on lines +42 to +43
Create a tuple of *size* items and copy references from *array* to the new
tuple.
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to formulate this in words more similar to PyTuple_New() or PyTuple_Pack() ("Return a new tuple object of size ... "), but this is not so important.

@encukou
Copy link
Member

encukou commented Oct 7, 2025

Implementation looks good, thanks! Do you want to open a WG vote for this?

@vstinner
Copy link
Member Author

vstinner commented Oct 7, 2025

Do you want to open a WG vote for this?

I created capi-workgroup/decisions#79.

@vstinner
Copy link
Member Author

vstinner commented Oct 7, 2025

I exchanged array and size parameters, new API:

PyObject* PyTuple_FromArray(Py_ssize_t size, PyObject *const *array)

@vstinner
Copy link
Member Author

vstinner commented Oct 8, 2025

I exchanged array and size parameters, new API: (...)

Sorry, I changed my preference back to (array, size) as before :-) See the API discussion at: capi-workgroup/decisions#79

@vstinner
Copy link
Member Author

I created capi-workgroup/decisions#79.

The C API Working Group accepted this API.

@vstinner vstinner merged commit e31c22d into python:main Oct 10, 2025
90 checks passed
@vstinner vstinner deleted the tuple_fromarray branch October 10, 2025 06:54
@vstinner
Copy link
Member Author

Merged. Thanks for your review @serhiy-storchaka and @encukou!

@vstinner
Copy link
Member Author

I added the function to pythoncapi-compat: python/pythoncapi-compat@89e023e

Sign up for free to 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