It seems mmap module is using inappropriate exception types. For example, if (! (PyString_Check(v)) ) { PyErr_SetString(PyExc_IndexError, "mmap slice assignment must be a string"); return -1; } I think this should be PyExc_TypeError. if (self->size >= pos && count > self->size - pos) { PyErr_SetString(PyExc_ValueError, "source or destination out of range"); return NULL; I think this is out of range, so PyExc_IndexError. Of course, there is the case difficult to determine which exception is suitable. For example, if Py_ssize_t is negative value, OverflowError or IndexError?
It is uncomfortable changing exception types have been a published API for over a decade, so I'll leave this one open for a bit so that others can comment. Most of the changes in the patch are correct and match their counterparts for lists and strings. The TypeError for slice deletion was correct as-is (it matches del 'abc'[1]). The IndexError on slice assignment may be correct as-is (I'll do more research). The from ValueError to SystemError is questionable. Otherwise, the patch looks to be mostly correct (I'll give it a more thorough review later).
Though the changes look like they would nicely harmonize mmap with other modules, I'm going to decline this patch. No one else has stepped forward to offer agreement and it likely isn't worth the disruption that would come from breaking existing code. Unfortunately, the time the fix-up an API is when it is proposed rather than years after the ship has already sailed. Charles-François, thank you for the patch.
History
Date
User
Action
Args
2022-04-11 14:56:46
admin
set
github: 49634
2016-11-23 06:57:38
rhettinger
set
status: open -> closedresolution: rejectedmessages: +
