Skip to content

/cpython

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. #17773

Open
wants to merge 2 commits into
base: master
from
Open

bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. #17773

wants to merge 2 commits into from
+37 −1

Conversation

@vsajip
Copy link
Member

vsajip commented Dec 31, 2019 •
edited by bedevere-bot

This uses the heuristic of assuming a named tuple is a subclass of
tuple with a _fields attribute. This change means that contents of
a named tuple wouldn't be converted - if a user wants to have
ConvertingTuple functionality from a namedtuple, they will have to
implement it themselves.

https://bugs.python.org/issue39142
@vsajip
bpo-39142: Avoid converting namedtuple instances to ConvertingTuple.
Loading status checks…
91d74aa 
This uses the heuristic of assuming a named tuple is a subclass of
tuple with a _fields attribute. This change means that contents of
a named tuple wouldn't be converted - if a user wants to have
ConvertingTuple functionality from a namedtuple, they will have to
implement it themselves.
@vsajip

This comment has been minimized.

Sign in to view
Copy link
Member Author

vsajip commented Dec 31, 2019

@tirkarthi - I would appreciate any comments you might have on this PR. Thanks for the analysis on the issue.
@vsajip
Added NEWS entry.
Loading status checks…
a3defc8
@vsajip vsajip removed the skip news label Dec 31, 2019
@corona10
corona10 reviewed Jan 1, 2020
View changes
Copy link
Member

corona10 left a comment

Just comment:
We don't have to care structseq type in this issue?
@vsajip

This comment has been minimized.

Sign in to view
Copy link
Member Author

vsajip commented Jan 1, 2020 •
edited by bedevere-bot

We don't have to care structseq type in this issue?

I would say not, because it's easy and common to create a namedtuple in Python but structseq is generally used by C API code to create analogous objects, so occurrences are fewer. If a use case comes up that requires structseq support, we can e.g. later add a check for n_fields, but there's no compelling reason to do it now, IMO. Also, [bpo-1820](https://bugs.python.org/issue1820) is still open, which is looking at the possibility of converging the structseq and namedtuple APIs.
@tirkarthi
tirkarthi approved these changes Jan 1, 2020
View changes
Copy link
Contributor

tirkarthi left a comment

Thanks @corona10 for bringing it up. I agree with @vsajip that namedtuple is more common and has a Python API with structseq used internally in posix and time module. If there is a report I guess we can add a similar check in future. PR LGTM. Thanks Vinay.
@corona10
corona10 approved these changes Jan 1, 2020
View changes
Copy link
Member

corona10 left a comment

@vsajip Thanks for the explanation :)
LGTM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tirkarthi tirkarthi

@corona10 corona10

Assignees
No one assigned
Labels
CLA signed awaiting core review type-bugfix
Projects
None yet
Milestone
No milestone
5 participants
@vsajip @tirkarthi @corona10 @the-knights-who-say-ni @bedevere-bot
You can’t perform that action at this time.