Issue31956
Created on 2017-11-06 11:13 by niki.spahiev, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| WIP-issue-31956 | Phaqui, 2017-11-12 23:05 | |||
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 4435 | closed | python-dev, 2017-11-17 07:27 | |
| PR 25059 | merged | ZackerySpytz, 2021-03-28 22:56 | |
| Messages (14) | |||
|---|---|---|---|
| msg305629 - (view) | Author: Николай Спахиев (niki.spahiev) | Date: 2017-11-06 11:13 | |
Sequence protocol specifies 2 optional argument for index method: seq.index(value, [start, [stop]]) array.index(value) needs start and stop arguments too. |
|||
| msg305643 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2017-11-06 15:12 | |
+1 |
|||
| msg305650 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-11-06 16:22 | |
Николай Спахиев: Are you only asking for the feature, or are you interested to work on a concrete patch? |
|||
| msg305758 - (view) | Author: Николай Спахиев (niki.spahiev) | Date: 2017-11-07 13:42 | |
I plan to work on a patch. |
|||
| msg306133 - (view) | Author: Anders Lorentsen (Phaqui) * | Date: 2017-11-12 23:05 | |
I decided to work on this, and I would like some review, as this would be my second contribution to cpython. Also, a general question: As I defined the start and end arguments Py_ssize_t, bigger indexes (more negative or more positive) than what can fit in Py_ssize_t will trigger an overflow error. This should be OK, though, as other functions with index arguments has them as Py_ssize_t - and getarrayitem() itself accepts a Py_ssize_t. Or? |
|||
| msg306143 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2017-11-13 06:57 | |
Just take list.index as an example. |
|||
| msg306150 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2017-11-13 08:34 | |
Anders Lorentsen: Py_ssize_t is the correct type for an index in the C language. It's not technically possible to create an array object larger than PY_SSIZE_T_MAX items :-) Serhiy> Just take list.index as an example. I concur with Serhiy :-) |
|||
| msg306178 - (view) | Author: Anders Lorentsen (Phaqui) * | Date: 2017-11-14 00:00 | |
Writing my tests, I originally looked at Lib/test/seq_tests.py. One test case uses indexes that are (+-)4*sys.maxsize. This does not fit in Py_ssize_t, and so these tests cause my array implementation to raise an overflow exception. A solution is of course to have the function take general objects instead, and then truncate them down to a number that can fit in Py_ssize_t if it's too negative or positive). But I concur. It seems more reasonable to stay consistent with the rest of the module, too. I'll look over the test code to make sure I test for every given scenario (or as many as I can think of), and prepare a PR for this, then :) |
|||
| msg330746 - (view) | Author: Ryan G. (Ryan G.) | Date: 2018-11-30 05:39 | |
This functionality is useful to me. Is this issue still alive? If not, how can I help? |
|||
| msg350405 - (view) | Author: Joannah Nanjekye (nanjekyejoannah) * ![]() |
Date: 2019-08-24 22:51 | |
There is a pending PR here : https://github.com/python/cpython/pull/4435 by phaqui. @phaqui do you want to finish your PR ? |
|||
| msg350619 - (view) | Author: Anders Lorentsen (Phaqui) * | Date: 2019-08-27 09:29 | |
As far as I can recall, the patch is generally speaking good to go. A number of discussions arose on various details, however. In any event, I'll take a look at it during the next few days. |
|||
| msg352499 - (view) | Author: Anders Lorentsen (Phaqui) * | Date: 2019-09-15 23:33 | |
I have actually managed to lost my local branch of this fix, though I assume I can just start another one, manually copy over the changes, somehow mark this current PR as cancelled, aborted, or in my option the best: "replaced/superseeded by: [new PR]". In any case, there were discussions that seem to be unresolved, allow me to summarize: * Document that index() raises ValueError when *value* is not found? > vstinner: We don't do this, remove this addition. > serhiy: Other index() methods does this. ---> My patch current does this. Who has final saying here? * 'start' and 'stop' arguments are not keyword arguments, and also not shown in the signature as '.. start=0 ..' for this very reason (it may make them look as keyword arguments). Also, this lines up with list.index() for consistency. Wishes about changing this for all index()-methods has been expressed, but it seems to be consensus on doing this in unison for all index()-methods at once, in a bigger change... So, what is currently in the PR is good enough for now, or? * Wording in documentation: Clarify that "the returned index is still relative to the start of the array, not the searched sub sequence" or not? * Comment in the code about checking the length of the array on each iteration? There were comments about it being "confusing" - and while I agree, the other index()-code for lists, does not comment on this. Again I followed the path of most consistency, but I did receive comments about this. Yes to descriptive comments, or not? ---- Generally speaking: In the end, all I really did was mimic how list.index() is both written and documented, and that's when discussions about issues related to that started occurring, and so I now remember that I halted my PR, waiting for these issues to be resolved. |
|||
| msg369732 - (view) | Author: Cheryl Sabella (cheryl.sabella) * ![]() |
Date: 2020-05-23 17:36 | |
I found this SO about reclaiming an orphaned pull request: https://stackoverflow.com/a/53383772 But you may still need to open a new PR for it. |
|||
| msg390070 - (view) | Author: Dong-hee Na (corona10) * ![]() |
Date: 2021-04-02 15:28 | |
New changeset afd12650580725ac598b2845384771c14c4f952e by Zackery Spytz in branch 'master': bpo-31956: Add start and stop parameters to array.index() (GH-25059) https://github.com/python/cpython/commit/afd12650580725ac598b2845384771c14c4f952e |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:54 | admin | set | github: 76137 |
| 2022-03-19 22:41:58 | iritkatriel | link | issue42884 superseder |
| 2021-04-09 01:26:32 | corona10 | set | status: open -> closed stage: patch review -> resolved resolution: fixed versions: + Python 3.10, - Python 3.7 |
| 2021-04-02 15:28:49 | corona10 | set | messages: + msg390070 |
| 2021-03-30 14:49:14 | corona10 | set | nosy:
+ corona10 |
| 2021-03-28 22:56:29 | ZackerySpytz | set | nosy:
+ ZackerySpytz pull_requests: + pull_request23807 |
| 2021-03-13 21:16:56 | vstinner | set | nosy:
- vstinner |
| 2021-03-13 19:20:26 | kamilturek | set | nosy:
+ kamilturek |
| 2020-05-23 17:36:06 | cheryl.sabella | set | nosy:
+ cheryl.sabella messages: + msg369732 |
| 2019-09-15 23:33:40 | Phaqui | set | messages: + msg352499 |
| 2019-08-27 09:29:22 | Phaqui | set | messages: + msg350619 |
| 2019-08-24 22:51:03 | nanjekyejoannah | set | nosy:
+ nanjekyejoannah messages: + msg350405 |
| 2018-12-15 09:33:42 | xtreak | link | issue35508 superseder |
| 2018-11-30 05:39:54 | Ryan G. | set | nosy:
+ Ryan G. messages: + msg330746 |
| 2017-11-17 07:27:58 | python-dev | set | keywords:
+ patch stage: patch review pull_requests: + pull_request4379 |
| 2017-11-14 00:00:39 | Phaqui | set | messages: + msg306178 |
| 2017-11-13 08:34:00 | vstinner | set | messages: + msg306150 |
| 2017-11-13 06:57:36 | serhiy.storchaka | set | versions:
+ Python 3.7, - Python 3.8 nosy: + serhiy.storchaka messages: + msg306143 components:
+ Extension Modules, - Library (Lib) |
| 2017-11-12 23:05:19 | Phaqui | set | files:
+ WIP-issue-31956 nosy: + Phaqui messages: + msg306133 |
| 2017-11-07 13:42:39 | niki.spahiev | set | messages: + msg305758 |
| 2017-11-06 16:22:06 | vstinner | set | nosy:
+ vstinner messages: + msg305650 |
| 2017-11-06 15:12:03 | rhettinger | set | nosy:
+ rhettinger messages: + msg305643 |
| 2017-11-06 11:13:47 | niki.spahiev | create | |
