bpo-5945: mapping.rst: PyMapping_Check(list) returns 1#144
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
Sorry, something went wrong.
|
Should the commit rebase into one commit? |
Sorry, something went wrong.
|
@grapherd The PR can be accepted as a squash merge in one click that will have the same effect as manually squashing it. |
Sorry, something went wrong.
As discussed below, in Python 3, PyMapping_Check(list) returns 1. This behavior is justified by a developer as consistent with Python 3's internals. It is however surprising to C-API users (speaking from experience) and because it deviates from collections.abc.Mapping. https://mail.python.org/pipermail/python-dev/2009-May/089445.html Squashed a few revisions
e9ad4cc to
0c7abc1
Compare
February 27, 2017 20:08
|
Rebased to HEAD and squashed |
Sorry, something went wrong.
|
I think this looks good now. @serhiy-storchaka would you like to take another look? I can do the backports. |
Sorry, something went wrong.
|
No backports please. |
Sorry, something went wrong.
|
This is just the documentation fix. I think it should be backported. But proposed wording doesn't LGTM. See my comment on the tracker. |
Sorry, something went wrong.
|
On bpo serhiy-storchaka wrote:
There is no recommendation to use
So I think it is appropriate to keep the proposed wording since it steers users to ABC checks and away from these APIs. If there is a change perhaps it should be explicitly "not recommended" in the documentation. |
Sorry, something went wrong.
Stackless contributes two tests to builtins: TaskletExit and TaskletExit.__init__. Therefore we have to adjust the limit. Add missing changelog entries (python#143, python#144).
|
To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request. If/when the requested changes have been made, please leave a comment that says, |
Sorry, something went wrong.
|
I have made the requested changes; please review again. Given that several core developers suggested or concurred with deprecating this API (Benjamin Peterson, GvR, Raymond Hettinger) I marked it not recommended as an alternative to officially deprecating. I think it is best to omit the suggestion to use |
Sorry, something went wrong.
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
Sorry, something went wrong.
Stackless contributes two tests to builtins: TaskletExit and TaskletExit.__init__. Therefore we have to adjust the limit. Add missing changelog entries (python#143, python#144). (cherry picked from commit 7327e4b)
willingc
left a comment
There was a problem hiding this comment.
@jbarlow83 Thanks for the PR and the changes. To keep this PR from further languishing, I suppose it may be most efficient to update the wording to incorporate @serhiy-storchaka's suggestion.
Sorry, something went wrong.
|
Thanks @jbarlow83 for the quick response. @serhiy-storchaka This looks good to me now. If you have no further changes, I recommend merging. Thanks. |
Sorry, something went wrong.
…/100 PURE
(D) implementation per theologian 00:02:12Z scoping audit (overturned A1
'classloader.h C++-only' framing as incomplete) + supervisor 00:04:33Z
hold-lift (terminal goal pre-authorized via memory line 9 ZERO-C++).
Phase 3D status: 99/100 PURE-CONVERTED + 1 PARTIAL (emitLoadMethodStatic) →
100/100 PURE-CONVERTED. Phase 3D close criteria as Alex memory-line-9
directive: pure C, no C++ compiler needed for emit body.
CHANGES:
(1) Python/jit/hir/builder.h: 2 friend declarations for new bridges.
- friend int ::hir_builder_preloader_invoke_method_slot_c(void*, PyObject*)
- friend void ::hir_builder_state_static_method_stack_push_cpp(void*, void*)
(2) Python/jit/hir/builder.cpp:
- HIRBuilder::emitLoadMethodStatic shrinks from 33 lines to 9 lines (pure
type marshaling: just calls C body with bc_instr.oparg() + code_).
- hir_builder_emit_load_method_static_c extern decl signature shrunk:
drops is_classmethod, vte_state_offset, vte_load_offset, is_static,
out_entry_func params; takes opcode oparg + code instead.
- 2 new bridge implementations:
- hir_builder_preloader_invoke_method_slot_c: returns slot from
invokeMethodTarget (was C++-only access via private preloader_).
- hir_builder_state_static_method_stack_push_cpp: pushes Register* to
static_method_stack_ (mirrors existing _pop_cpp bridge).
(3) Python/jit/hir/builder_emit_c.c:
- #include cinderx/StaticPython/classloader.h (transitively pulls
vtable.h with _PyType_VTable + _PyType_VTableEntry struct defs).
A1 framing was incomplete: vtable.h is already extern "C" wrapped +
pure typedef struct (no class/template/namespace), safe in C TU.
- Forward-decl 3 bridges used in C body.
- Body adds C-side derivations (was C++ stub responsibility):
* arg = PyTuple_GET_ITEM(code->co_consts, oparg) (constArg equivalent)
* descr = PyTuple_GET_ITEM(arg, 0)
* is_classmethod = _PyClassLoader_IsClassMethodDescr(arg)
* slot = hir_builder_preloader_invoke_method_slot_c(builder, descr)
* is_static = via existing hir_builder_invoke_method_target_c bridge
* vte_state/load_offset = offsetof + slot arithmetic, wrapped in
VTableByteOffset (P5 wrapper from push 58)
- Conditional static_method_stack push moved from C++ stub to C body
via hir_builder_state_static_method_stack_push_cpp bridge.
- .v unwrap at the 2 boundaries to hir_c_create_load_field_reg
(raw intptr_t API).
VERIFICATION pending: testkeeper rebuild + 30x all 8 W-2A+W-2B sentinels
(must remain 240/240 PASS, behavior-equivalent expected) + Phoenix suite
+ ABBA + dual-arch + INVOKE_*-style structural test
(test_phoenix_partial_conversions.py).
Per pythia python#144 python#2 + supervisor 00:04:33Z: Alex notification will follow
push 60 landing ('Phase 3D 100/100 PURE achieved via (D) — minimal scope
~30-60min, terminal goal directly served per memory line 9'). Per
feedback_dont_ask_just_do.md: terminal goal already authorized; (D) is
implementation path serving that authorization, not new scope.
Auth chain: theologian scoping audit 00:02:12Z + theologian go-direct
00:03:38Z + supervisor hold-lift 00:04:33Z + generalist (D) feasibility
verification 00:02:38Z.
As discussed below, in Python 3, PyMapping_Check(list) returns 1. This behavior is justified by a developer as consistent with Python 3's internals. It is however surprising to C-API users (speaking from experience) and because it deviates from collections.abc.Mapping.
I think it is better to be vague than describe the implementation details.
https://mail.python.org/pipermail/python-dev/2009-May/089445.html
https://bugs.python.org/issue5945