Title: typos in curses argument error messages
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 2.7
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, chris.jerdonek, ezio.melotti, pconnell, r.david.murray, serhiy.storchaka
Priority: normal Keywords: easy, needs review, patch

Created on 2012-09-02 21:41 by chris.jerdonek, last changed 2017-12-21 07:29 by serhiy.storchaka. This issue is now closed.

File name Uploaded Description Edit
issue-15852-1.patch chris.jerdonek, 2012-09-03 02:08 review
Pull Requests
URL Status Linked Edit
PR 4950 merged thatiparthy, 2017-12-20 20:40
PR 4952 merged python-dev, 2017-12-21 05:42
PR 4951 merged python-dev, 2017-12-21 05:43
Messages (12)
msg169725 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-09-02 21:41
It seems like a couple error messages in the curses module need correcting:

PyErr_SetString(PyExc_TypeError, "insch requires 1 or 4 arguments");

PyErr_SetString(PyExc_TypeError, "instr requires 0 or 3 arguments");

In both cases, "or" should be "to".
msg169727 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-09-02 23:12
Given their signatures in the docs, I suspect it is more complicated than that.  Perhaps the error messages are even correct.  What does the code implement?
msg169729 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-09-02 23:24
I just completed a patch to improve the documentation of these signatures (along with many others) in issue 15831.  The correction here is consistent with my findings and revised documentation there.

As for the code, they are straightforward switch statements similar to many of the other methods in that module.  I also did a manual test on one of the methods as a sanity check.  I am preparing unit tests.
msg169730 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-09-02 23:28
The situation is the same in 2.7 (and probably 3.2).
msg169736 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-09-03 02:06
Attaching a patch with tests and fix for the default branch.
msg169737 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-09-03 02:08
Uploading correct file.
msg170871 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-09-21 09:10
Can't you use assertRaisesRegex?
msg170900 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-09-21 16:32
Thanks for taking a look at this, and good question.

Without restructuring how the tests are done, I believe the short answer is no.  The funny thing about this test module is that it does not actually have any unittest test cases.  It just calls some functions.  Failure happens if an exception is raised in any one of those functions.

See here, for example:
msg170927 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-09-21 21:33
FYI, I created issue 16000 :) to switch test_curses to using unittest.TestCase.
msg187287 - (view) Author: Phil Connell (pconnell) * Date: 2013-04-18 20:14
The patch looks correct and complete, and still patches and passes the tests.

So, as far as I can see, this can be committed.
msg305388 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-01 18:03
Do you mind to create a pull request on GitHub Chris?

In general the patch LGTM, but I don't think this minor typo fix needs tests for exact error messages. Adding new tests is good, but I think it is enough to test that corresponding functions accept the correct number of arguments and raise TypeError on incorrect number of arguments.
msg308857 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-12-21 07:29
It was worth to honor Chris as the author of the patch in the commit message.
Date User Action Args
2017-12-21 07:29:59serhiy.storchakasetmessages: + msg308857
2017-12-21 05:44:03asvetlovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-12-21 05:43:06python-devsetpull_requests: + pull_request4847
2017-12-21 05:42:35python-devsetpull_requests: + pull_request4846
2017-12-20 20:40:16thatiparthysetpull_requests: + pull_request4843
2017-11-01 18:03:03serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg305388
versions: + Python 3.6, Python 3.7, - Python 3.4, Python 3.5
2015-01-26 06:34:35berker.peksagsetnosy: + berker.peksag

versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3
2013-04-18 20:14:27pconnellsetnosy: + pconnell
messages: + msg187287
2012-09-21 21:33:05chris.jerdoneksetmessages: + msg170927
2012-09-21 16:32:59chris.jerdoneksetmessages: + msg170900
2012-09-21 09:10:10ezio.melottisetmessages: + msg170871
2012-09-08 15:07:18ezio.melottisetnosy: + ezio.melotti
2012-09-03 02:10:16chris.jerdoneksetfiles: - issue-15831-1.patch
2012-09-03 02:08:38chris.jerdoneksetfiles: + issue-15852-1.patch

messages: + msg169737
2012-09-03 02:06:35chris.jerdoneksetkeywords: + needs review
stage: test needed -> patch review
2012-09-03 02:06:17chris.jerdoneksetfiles: + issue-15831-1.patch
keywords: + patch
messages: + msg169736
2012-09-02 23:28:46chris.jerdoneksetmessages: + msg169730
versions: + Python 2.7, Python 3.2
2012-09-02 23:24:43chris.jerdoneksetmessages: + msg169729
2012-09-02 23:12:11r.david.murraysetnosy: + r.david.murray
messages: + msg169727
2012-09-02 21:41:42chris.jerdonekcreate