This new patch works for keys(), .items(), and .values()  (as intended by
the original change)


On Wed, Jul 1, 2020 at 5:27 PM Stestagg <stest...@gmail.com> wrote:

> For what it's worth, I've attached a patch that implements this as a
> prototype.
>
> It's definitely not mergeable right now.  I'm not aware of any issues, but
> the code needs more error checking, and the coding style doesn't match
> cpython's.
>
> Having said that, it's a 70-line change, so fixing that up should be
> trivial once the change has some agreement behind it.
>
> Steve
>
>
>
> On Wed, Jul 1, 2020 at 4:31 PM Stestagg <stest...@gmail.com> wrote:
>
>> I'll try to unwind the rabbit holes a bit and suggest that the
>> differences in opinion here boil down to:
>>
>> Is it most useful to consider dict_keys/items/values as: (This doesn't
>> mean what isinstance() returns, or what a previous pep has stated, just how
>> one thinks about them):
>>  1. ordered collections that have some set-like operators added for
>> convenience, (conceptually, if not exactly, class ...(Set, Sequence):)  but
>> are missing a single method full Sequence interface (__getitem__)
>> or
>>  2. Sets that happen to have a stable/deterministic element order
>>
>> My opinion is that, as of Python 3.7, they are effectively 1, even if the
>> isinstance hooks haven't been updated.  I can see why people may think of
>> them as 2. I don't have any desire to change minds on this :)
>>
>>
>> Semantic quibbling aside, My opinions/reasoning on the different options
>> are the following:
>>
>>
>> * Do nothing:
>> -0.5: I can live with the current situation, some things I do
>> infrequently will be slightly less convenient, but can work around easily.
>>
>> * Add numeric `__getitem__` to `dict_*` view classes:
>> +1  - This restores code that used to work in python 2, and makese some
>> things a bit easier.  The O(n) behaviour is not ideal, but in my opinion is
>> an acceptable compromise
>>
>> * Add new method to dict class:
>> -1 This doesn't feel like a good solution to me.  Rather than continue to
>> argue about the justification for why, let's just say its a personal
>> judgement
>>
>> I'm interested in any other opinions on these options
>>
>> Steve
>>
>>
>>
>> On Wed, Jul 1, 2020 at 3:24 PM Steven D'Aprano <st...@pearwood.info>
>> wrote:
>>
>>> On Wed, Jul 01, 2020 at 01:36:34PM +0100, Stestagg wrote:
>>> > On Wed, Jul 1, 2020 at 12:18 PM Steven D'Aprano <st...@pearwood.info>
>>> wrote:
>>> >
>>> > > On Tue, Jun 30, 2020 at 10:18:34AM +0100, Stestagg wrote:
>>> > >
>>> > > > This property is nice, as .keys() (for example) can operate like a
>>> > > > set and give lots of convenience features.  However this doesn't
>>> > > > really mean that these objects are trying to *be* sets in any
>>> > > > meaning way,
>>> > >
>>> > > Please read the PEP:
>>> > >
>>> > > https://www.python.org/dev/peps/pep-3106/
>>> > >
>>> > > It was an explicit design choice to make them be set-like, not an
>>> > > accident. The whole point was to offer a set interface.
>>> >
>>> >
>>> > I'm pretty familiar with that pep, I'm not sure how knowledge of its
>>> > contents affects this idea/discussion in any material way.
>>>
>>> You: I don't think that dict views are trying to be sets.
>>>
>>> Me: Dict views were designed right from the start to be like sets, as
>>> described in the PEP.
>>>
>>> You: I don't think the PEP is relevant.
>>>
>>> Okay, whatever you say.
>>>
>>>
>>> > The point being made,  was that the utility of trying to shoehorn the
>>> > dict_* types into a Set pidgeon-hole is quite limited, as
>>> practicalities
>>> > get in the way.
>>>
>>> *shrug*
>>>
>>> They seem to work pretty well for many uses. Perhaps they could be
>>> better, but they are what they are, and there's no point pretending that
>>> they aren't large-s Sets when they clearly are.
>>>
>>>     py> from collections.abc import Set
>>>     py> isinstance({}.items(), Set)
>>>     True
>>>
>>> This is not an accident, it is not a mistake, it wasn't forced on the
>>> language because dicts used to be unordered. It was an intentional
>>> design choice.
>>>
>>> If you think that *dict views are set-like* being an intentional choice
>>> is not important to the discussion, what do you consider important?
>>>
>>> As far as I am concerned, making dict views into sequences by making
>>> them indexable should be ruled out. They were intended to be set-like,
>>> not sequence-like, and that is the end of the story. Adding indexing
>>> onto them is bolting on an API that was never part of the design.
>>>
>>> I still don't think there are any compelling use-cases for indexing
>>> dicts, but if there are, we should offer a getter method on the dict
>>> object itself, not try to turn dict views into a hybrid set+sequence.
>>> Change my mind.
>>>
>>>
>>> > > > I don't think inventing new methods for emulating __getitem__ is
>>> that
>>> > > > useful here, there is a well established standard for getting
>>> items from
>>> > > > a container by index, I think we should use it :).
>>> > >
>>> > > We can't use dict[index] because the index can also be a key:
>>> > >
>>> > >     d = {'a': None, 'b': None, 0: 999}
>>> > >     d[0]  # return 'a' or 999?
>>> >
>>> > The currently discussed option is for adding`__getitem__` to the
>>> > dict_keys/dict_items/dict_values types, *NOT* the base `dict` type.
>>>
>>> You: We shouldn't invent a new way of indexing containers.
>>>
>>> Also you: Let's invent a new way of indexing containers (subscripting on
>>> dict views).
>>>
>>> So which is it?
>>>
>>> Subscripting on the dict object is ruled out because it is ambiguous.
>>> Here are three possible choices (there may be others):
>>>
>>> - Do nothing, we don't need this functionality, there are no compelling
>>>   use-cases for it. Passing dict keys to random.choice is not a
>>>   compelling use-case.
>>>
>>> - Add a new interface by making dict views a hybrid set+sequence.
>>>
>>> - Add a new getter method on the dict itself.
>>>
>>> Aside from the first do-nothing option, one way or the other we're
>>> adding a new way of indexing the object we actually want to index,
>>> namely the dict.
>>>
>>> The choice is between a getter interface, and a subscript interface on a
>>> proxy (a dict view). Dicts already offer getter interfaces (dict.get,
>>> dict.setdefault) so it is not like calling dict methods is unheard of.
>>>
>>>
>>> [...]
>>> > The point wasn't that: "It's atypical so we can ignore it", but rather:
>>> > "It's faster/better, but seldom available to be used, here's how we do
>>> it
>>> > for the more common case..."
>>>
>>> Okay, thanks for the correction.
>>>
>>>
>>> --
>>> Steven
>>> _______________________________________________
>>> Python-ideas mailing list -- python-ideas@python.org
>>> To unsubscribe send an email to python-ideas-le...@python.org
>>> https://mail.python.org/mailman3/lists/python-ideas.python.org/
>>> Message archived at
>>> https://mail.python.org/archives/list/python-ideas@python.org/message/A34NF6R6JTSQAZLIU6IKN4UWC2L2WANO/
>>> Code of Conduct: http://python.org/psf/codeofconduct/
>>>
>>
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index b1f11b3e69..b1b6c30466 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -4107,6 +4107,79 @@ dictview_len(_PyDictViewObject *dv)
     return len;
 }
 
+
+PyObject * _dictview_getitem_ma_unsafe(_PyDictViewObject *dv, size_t index) {
+    PyObject *key, *value, *result;
+    PyDictObject *dict = dv->dv_dict;
+
+    if (PyDictKeys_Check(dv)) {
+        result = DK_ENTRIES(dict->ma_keys)[index].me_key;
+        Py_INCREF(result);
+    }
+    else if (PyDictValues_Check(dv))
+    {
+        result = dict->ma_values[index];
+        Py_INCREF(result);
+    } else {
+        result = PyTuple_New(2);
+        key = DK_ENTRIES(dict->ma_keys)[index].me_key;
+        value = dict->ma_values[index];
+        Py_INCREF(key);
+        Py_INCREF(value);
+        PyTuple_SET_ITEM(result, 0, key);
+        PyTuple_SET_ITEM(result, 1, value);
+    }
+    return result;
+}
+
+static PyObject *dv_indexerr = NULL;
+
+PyObject * dictview_getitem(_PyDictViewObject *dv, Py_ssize_t index)
+{
+    PyObject *result;
+    PyDictObject *dict = dv->dv_dict;
+
+    if ((size_t) index >= (size_t) dict->ma_used) {
+        if (dv_indexerr == NULL) {
+            dv_indexerr = PyUnicode_FromString(
+                "index out of range");
+            if (dv_indexerr == NULL)
+                return NULL;
+        }
+        PyErr_SetObject(PyExc_IndexError, dv_indexerr);
+        return NULL;
+    }
+
+    if (_PyDict_HasSplitTable(dict)) {
+        return _dictview_getitem_ma_unsafe(dv, index);
+    }
+
+    PyDictKeyEntry *entry_ptr = DK_ENTRIES(dict->ma_keys);
+    while(index) {
+        if (entry_ptr != NULL) {
+            index--;
+        }
+        entry_ptr++;
+    }
+
+    if (PyDictKeys_Check(dv)) {
+        result = entry_ptr->me_key;
+        Py_INCREF(result);
+    }
+    else if (PyDictValues_Check(dv))
+    {
+        result = entry_ptr->me_value;
+        Py_INCREF(result);
+    } else {
+        result = PyTuple_New(2);
+        Py_INCREF(entry_ptr->me_key);
+        Py_INCREF(entry_ptr->me_value);
+        PyTuple_SetItem(result, 0, entry_ptr->me_key);
+        PyTuple_SetItem(result, 1, entry_ptr->me_value);
+    }
+    return result;
+}
+
 PyObject *
 _PyDictView_New(PyObject *dict, PyTypeObject *type)
 {
@@ -4287,7 +4360,7 @@ static PySequenceMethods dictkeys_as_sequence = {
     (lenfunc)dictview_len,              /* sq_length */
     0,                                  /* sq_concat */
     0,                                  /* sq_repeat */
-    0,                                  /* sq_item */
+    (ssizeargfunc)dictview_getitem,     /* sq_item */
     0,                                  /* sq_slice */
     0,                                  /* sq_ass_item */
     0,                                  /* sq_ass_slice */
@@ -4716,7 +4789,7 @@ static PySequenceMethods dictitems_as_sequence = {
     (lenfunc)dictview_len,              /* sq_length */
     0,                                  /* sq_concat */
     0,                                  /* sq_repeat */
-    0,                                  /* sq_item */
+    (ssizeargfunc)dictview_getitem,     /* sq_item */
     0,                                  /* sq_slice */
     0,                                  /* sq_ass_item */
     0,                                  /* sq_ass_slice */
@@ -4799,7 +4872,7 @@ static PySequenceMethods dictvalues_as_sequence = {
     (lenfunc)dictview_len,              /* sq_length */
     0,                                  /* sq_concat */
     0,                                  /* sq_repeat */
-    0,                                  /* sq_item */
+    (ssizeargfunc)dictview_getitem,     /* sq_item */
     0,                                  /* sq_slice */
     0,                                  /* sq_ass_item */
     0,                                  /* sq_ass_slice */
_______________________________________________
Python-ideas mailing list -- python-ideas@python.org
To unsubscribe send an email to python-ideas-le...@python.org
https://mail.python.org/mailman3/lists/python-ideas.python.org/
Message archived at 
https://mail.python.org/archives/list/python-ideas@python.org/message/ND3DET6EI4IPVD6LDX5SDLFFOWSUM7DX/
Code of Conduct: http://python.org/psf/codeofconduct/

Reply via email to