GeezerVison is showing github.com — gh-109543: Remove unnecessary hasattr check by JelleZijlstra · Pull Request #109544 · python/cpython
Conversation
JelleZijlstra
commented
Sep 18, 2023
•
edited by bedevere-app
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Also added a new test case covering the scenario I thought this
might be about.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Also added a new test case covering the scenario I thought this
might be about.
|
|
class TD2(TD1):
|
|
|
b: str
|
|
|
|
|
|
self.assertIs(TD2.__total__, True)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you're not changing behaviour here, but are the precise semantics here specified anywhere? Mypy has some interesting behaviour here -- it treats all the keys present in TD2 as required, but all the keys present in TD1 as not required. That implies to me that TD2 should not be seen as a "total" TypedDict.
https://mypy-play.net/?mypy=latest&python=3.11&gist=e7ef88dd8c2b5697297c4739d471ac45
The __total__ attribute seems to be pretty meaningless these days, though, in a post-PEP-655 world:
>>> from typing import *
>>> class Foo(TypedDict):
... x: Required[int]
... y: NotRequired[str]
...
>>> Foo.__total__
True Maybe we should treat that as a bug and make it meaningful. Or maybe we should just be clearer in the documentation that __total__ doesn't indicate whether or not all keys in the TypedDict are required or not, it just literally indicates whether that specific TypedDict was constructed using total=True (but I'm not sure why that would be useful to anybody).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mypy behavior is right; that was the way to mix Required and NotRequired keys before PEP 655.
Agree that __total__ isn't that useful and you should generally look at __required_keys__ and __optional_keys__. However, I think the current behavior is right. If you do want to use __total__ for introspection, you should look not just at the current class's attribute, but also those of base classes.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's strange to me that we iterate through the base classes as part of the class construction in order to make sure we have accurate __required_keys__, __optional_keys__ and __annotations__ attributes on the produced class, but we don't do the same for the __total__ attribute. (We probably shouldn't be doing that for the __annotations__ attribute, since that's not the way __annotations__ works on any other class... but we do it anyway.) It seems very inconsistent to me.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Submitted #109547 with some enhancements to the docs.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semantics of this attribute seem pretty confusing -- I'm honestly wondering whether it might not be better to deprecate it. But I'm pretty confident you're correct that there's no way for __total__ to already be set at this point.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
JelleZijlstra
added a commit
to JelleZijlstra/cpython
that referenced
this pull request
Sep 18, 2023
As discussed in comments to python#109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot s390x SLES 3.x has failed when building commit 1293fcc.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/540/builds/6580) and take a look at the build logs.
- Check if the failure is related to this commit (1293fcc) or if it is a false positive.
- 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/540/builds/6580
Failed tests:
- test.test_asyncio.test_subprocess
Failed subtests:
- test_subprocess_consistent_callbacks - test.test_asyncio.test_subprocess.SubprocessThreadedWatcherTests.test_subprocess_consistent_callbacks
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-sles-z/build/Lib/test/test_asyncio/test_subprocess.py", line 788, in test_subprocess_consistent_callbacks
self.loop.run_until_complete(main())
File "/home/dje/cpython-buildarea/3.x.edelsohn-sles-z/build/Lib/asyncio/base_events.py", line 664, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "/home/dje/cpython-buildarea/3.x.edelsohn-sles-z/build/Lib/test/test_asyncio/test_subprocess.py", line 780, in main
self.assertEqual(events, [
AssertionError: Lists differ: [('pi[29 chars]t'), 'pipe_connection_lost', ('pipe_data_recei[57 chars]ted'] != [('pi[29 chars]t'), ('pipe_data_received', 2, b'stderr'), 'pi[57 chars]ted']
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot s390x RHEL7 LTO 3.x has failed when building commit 1293fcc.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/402/builds/5427) and take a look at the build logs.
- Check if the failure is related to this commit (1293fcc) or if it is a false positive.
- 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/402/builds/5427
Failed tests:
- test.test_asyncio.test_subprocess
Failed subtests:
- test_subprocess_consistent_callbacks - test.test_asyncio.test_subprocess.SubprocessThreadedWatcherTests.test_subprocess_consistent_callbacks
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto/build/Lib/test/test_asyncio/test_subprocess.py", line 788, in test_subprocess_consistent_callbacks
self.loop.run_until_complete(main())
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto/build/Lib/asyncio/base_events.py", line 664, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto/build/Lib/test/test_asyncio/test_subprocess.py", line 780, in main
self.assertEqual(events, [
AssertionError: Lists differ: [('pi[29 chars]t'), 'pipe_connection_lost', ('pipe_data_recei[57 chars]ted'] != [('pi[29 chars]t'), ('pipe_data_received', 2, b'stderr'), 'pi[57 chars]ted']
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot ARM Raspbian 3.x has failed when building commit 1293fcc.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/424/builds/4967) and take a look at the build logs.
- Check if the failure is related to this commit (1293fcc) or if it is a false positive.
- 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/424/builds/4967
Summary of the results of the build (if available):
==
Click to see traceback logs
remote: Enumerating objects: 20, done.
remote: Counting objects: 5% (1/20)
remote: Counting objects: 10% (2/20)
remote: Counting objects: 15% (3/20)
remote: Counting objects: 20% (4/20)
remote: Counting objects: 25% (5/20)
remote: Counting objects: 30% (6/20)
remote: Counting objects: 35% (7/20)
remote: Counting objects: 40% (8/20)
remote: Counting objects: 45% (9/20)
remote: Counting objects: 50% (10/20)
remote: Counting objects: 55% (11/20)
remote: Counting objects: 60% (12/20)
remote: Counting objects: 65% (13/20)
remote: Counting objects: 70% (14/20)
remote: Counting objects: 75% (15/20)
remote: Counting objects: 80% (16/20)
remote: Counting objects: 85% (17/20)
remote: Counting objects: 90% (18/20)
remote: Counting objects: 95% (19/20)
remote: Counting objects: 100% (20/20)
remote: Counting objects: 100% (20/20), done.
remote: Compressing objects: 9% (1/11)
remote: Compressing objects: 18% (2/11)
remote: Compressing objects: 27% (3/11)
remote: Compressing objects: 36% (4/11)
remote: Compressing objects: 45% (5/11)
remote: Compressing objects: 54% (6/11)
remote: Compressing objects: 63% (7/11)
remote: Compressing objects: 72% (8/11)
remote: Compressing objects: 81% (9/11)
remote: Compressing objects: 90% (10/11)
remote: Compressing objects: 100% (11/11)
remote: Compressing objects: 100% (11/11), done.
remote: Total 11 (delta 9), reused 1 (delta 0), pack-reused 0
From https://github.com/python/cpython
* branch main -> FETCH_HEAD
Note: switching to '1293fcc3c6b67b7e8d0081863ec6387e162341eb'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 1293fcc3c6 gh-109543: Remove unnecessary hasattr check (#109544)
Switched to and reset branch 'main'
./Modules/_testcapi/heaptype_relative.c: In function ‘make_sized_heaptypes’:
./Modules/_testcapi/heaptype_relative.c:60:28: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
60 | (unsigned long long)data_ptr,
| ^
make: *** [Makefile:2036: buildbottest] Error 3
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot PPC64LE RHEL7 3.x has failed when building commit 1293fcc.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/446/builds/3915) and take a look at the build logs.
- Check if the failure is related to this commit (1293fcc) or if it is a false positive.
- 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/446/builds/3915
Failed tests:
Failed subtests:
- testSendmsgTimeout - test.test_socket.SendmsgTCPTest.testSendmsgTimeout
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-ppc64le/build/Lib/test/test_socket.py", line 3145, in testSendmsgTimeout
self.assertTrue(self.misc_event.wait(timeout=self.fail_timeout))
AssertionError: False is not true
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-ppc64le/build/Lib/test/test_socket.py", line 365, in raise_queued_exception
raise self.queue.get()
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-ppc64le/build/Lib/test/test_socket.py", line 402, in clientRun
test_func()
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-ppc64le/build/Lib/test/test_socket.py", line 3152, in _testSendmsgTimeout
self.sendmsgToServer([b"a"*512])
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-ppc64le/build/Lib/test/test_socket.py", line 2840, in sendmsgToServer
return self.cli_sock.sendmsg(
^^^^^^^^^^^^^^^^^^^^^^
ConnectionResetError: [Errno 104] Connection reset by peer
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot PPC64LE Fedora Stable LTO 3.x has failed when building commit 1293fcc.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/541/builds/3401) and take a look at the build logs.
- Check if the failure is related to this commit (1293fcc) or if it is a false positive.
- 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/541/builds/3401
Failed tests:
Failed subtests:
- test_zippath_from_non_installed_posix - test.test_venv.BasicTest.test_zippath_from_non_installed_posix
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le.lto/build/Lib/test/test_venv.py", line 578, in test_zippath_from_non_installed_posix
shutil.copytree(fn, os.path.join(libdir, name))
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le.lto/build/Lib/shutil.py", line 588, in copytree
return _copytree(entries=entries, src=src, dst=dst, symlinks=symlinks,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le.lto/build/Lib/shutil.py", line 542, in _copytree
raise Error(errors)
shutil.Error: [('/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le.lto/build/Lib/test/test_future_stmt/__pycache__/import_nested_scope_twice.cpython-313.pyc.140735998680240', '/tmp/test_python_vy4zhu8t/tmpp6uaij6t/lib/python3.13/test/test_future_stmt/__pycache__/import_nested_scope_twice.cpython-313.pyc.140735998680240', "[Errno 2] No such file or directory: '/home/buildbot/buildarea/3.x.cstratak-fedora-stable-ppc64le.lto/build/Lib/test/test_future_stmt/__pycache__/import_nested_scope_twice.cpython-313.pyc.140735998680240'")]
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot PPC64LE RHEL8 LTO 3.x has failed when building commit 1293fcc.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/361/builds/4077) and take a look at the build logs.
- Check if the failure is related to this commit (1293fcc) or if it is a false positive.
- 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/361/builds/4077
Failed tests:
- test.test_asyncio.test_unix_events
Failed subtests:
- test_fork_signal_handling - test.test_asyncio.test_unix_events.TestFork.test_fork_signal_handling
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto/build/Lib/unittest/async_case.py", line 90, in _callTestMethod
if self._callMaybeAsync(method) is not None:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto/build/Lib/unittest/async_case.py", line 117, in _callMaybeAsync
return self._asyncioTestContext.run(func, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto/build/Lib/test/support/hashlib_helper.py", line 49, in wrapper
return func_or_class(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le.lto/build/Lib/test/test_asyncio/test_unix_events.py", line 1937, in test_fork_signal_handling
self.assertTrue(child_handled.is_set())
AssertionError: False is not true
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
JelleZijlstra
added a commit
that referenced
this pull request
Sep 27, 2023
As discussed in comments to #109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
Sep 27, 2023
As discussed in comments to pythonGH-109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
(cherry picked from commit f49958c)
Co-authored-by: Jelle Zijlstra <[email protected]>
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
Sep 27, 2023
As discussed in comments to pythonGH-109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
(cherry picked from commit f49958c)
Co-authored-by: Jelle Zijlstra <[email protected]>
JelleZijlstra
added a commit
that referenced
this pull request
Sep 27, 2023
… (#109983)
As discussed in comments to GH-109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
(cherry picked from commit f49958c)
Co-authored-by: Jelle Zijlstra <[email protected]>
csm10495
pushed a commit
to csm10495/cpython
that referenced
this pull request
Sep 28, 2023
Also added a new test case covering the scenario I thought this
might be about.
csm10495
pushed a commit
to csm10495/cpython
that referenced
this pull request
Sep 28, 2023
As discussed in comments to python#109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
vstinner
pushed a commit
that referenced
this pull request
Oct 4, 2023
… (#109982)
Enhance TypedDict docs around required/optional keys (GH-109547)
As discussed in comments to GH-109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
(cherry picked from commit f49958c)
Co-authored-by: Jelle Zijlstra <[email protected]>
Glyphack
pushed a commit
to Glyphack/cpython
that referenced
this pull request
Sep 2, 2024
Also added a new test case covering the scenario I thought this
might be about.
Glyphack
pushed a commit
to Glyphack/cpython
that referenced
this pull request
Sep 2, 2024
As discussed in comments to python#109544, the semantics of this attribute
are somewhat confusing. Add a note explaining its limitations and
steering users towards __required_keys__ and __optional_keys__ instead.
Daraan
added a commit
to Daraan/typing_extensions
that referenced
this pull request
Feb 22, 2025
Daraan
added a commit
to Daraan/typing_extensions
that referenced
this pull request
Feb 22, 2025
Daraan
added a commit
to Daraan/typing_extensions
that referenced
this pull request
Feb 22, 2025
Reviewers
gvanrossum
Awaiting requested review from gvanrossum
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|
AlexWaygood
left a comment