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

Merged
merged 2 commits into from Jan 1, 2020
Merged

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

merged 2 commits into from Jan 1, 2020
+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
@vsajip vsajip merged commit 46abfc1 into python:master Jan 1, 2020
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20191231.23 succeeded
Details
bedevere/issue-number Issue number 39142 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vsajip vsajip deleted the vsajip:fix-39142 branch Jan 1, 2020
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Jan 1, 2020

Thanks @vsajip for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 1, 2020

GH-17785 is a backport of this pull request to the 3.8 branch.
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Jan 1, 2020

Thanks @vsajip for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 1, 2020

GH-17786 is a backport of this pull request to the 3.7 branch.
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jan 1, 2020
@vsajip @miss-islington
bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (p…
d8f7fa3 
…ythonGH-17773)

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.
(cherry picked from commit 46abfc1)

Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
vsajip added a commit that referenced this pull request Jan 1, 2020
@miss-islington @vsajip
bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (G…
Loading status checks…
1d5a7e5 
…H-17773) (GH-17785)

(cherry picked from commit 46abfc1)
vsajip added a commit that referenced this pull request Jan 1, 2020
@miss-islington @vsajip
bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (G…
Loading status checks…
0e0e4ac 
…H-17773) (GH-17786)

(cherry picked from commit 46abfc1)
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 2, 2020

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86-64 High Sierra 3.7 has failed when building commit 0e0e4ac.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/139/builds/34) and take a look at the build logs.
  4. Check if the failure is related to this commit (0e0e4ac) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/139/builds/34

Summary of the results of the build (if available):

Click to see traceback logs 
remote: Enumerating objects: 14, done.        
remote: Counting objects:   7% (1/14)        
remote: Counting objects:  14% (2/14)        
remote: Counting objects:  21% (3/14)        
remote: Counting objects:  28% (4/14)        
remote: Counting objects:  35% (5/14)        
remote: Counting objects:  42% (6/14)        
remote: Counting objects:  50% (7/14)        
remote: Counting objects:  57% (8/14)        
remote: Counting objects:  64% (9/14)        
remote: Counting objects:  71% (10/14)        
remote: Counting objects:  78% (11/14)        
remote: Counting objects:  85% (12/14)        
remote: Counting objects:  92% (13/14)        
remote: Counting objects: 100% (14/14)        
remote: Counting objects: 100% (14/14), done.        
remote: Total 17 (delta 13), reused 13 (delta 13), pack-reused 3        
From https://github.com/python/cpython
 * branch                  3.7        -> FETCH_HEAD
Reset branch '3.7'

Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libpython3.7dm.a(dynamic_annotations.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libpython3.7dm.a(pymath.o) has no symbols
ld: warning: text-based stub file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation.tbd and library file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation.tbd and library file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation are out of sync. Falling back to library file for linking.
rror: [Errno 2] No such file or directory: '/Users/buildbot/buildarea/3.7.billenstein-sierra/build/target/include/python3.7dm/pyconfig.h'
make: *** [sharedmods] Error 1
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 type-bugfix
Projects
None yet
Milestone
No milestone
6 participants
@vsajip @miss-islington @bedevere-bot @tirkarthi @corona10 @the-knights-who-say-ni
You can’t perform that action at this time.