[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Fixed. Thank you for report, John.

--
resolution:  -> fixed
stage:  -> committed/rejected
status: open -> closed
versions: +Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Roundup Robot]

Roundup Robot added the comment:

New changeset 1b68dc917321 by Serhiy Storchaka in branch '3.3':
Issue #15972: Fix error messages when os functions expecting a file name or
http://hg.python.org/cpython/rev/1b68dc917321

New changeset 71fb426ee972 by Serhiy Storchaka in branch 'default':
Issue #15972: Fix error messages when os functions expecting a file name or
http://hg.python.org/cpython/rev/71fb426ee972

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Georg Brandl]

Georg Brandl added the comment:

This certainly isn't a release blocker.  Check it into default, and it will go 
into 3.3.1.

--
priority: release blocker -> normal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Larry Hastings]

Larry Hastings added the comment:

Georg: this okay to check in?  It passes the regression test.

--
nosy: +georg.brandl

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> Serhiy, you have the commit bit?

It is zero.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Larry Hastings]

Larry Hastings added the comment:

LGTM.  Serhiy, you have the commit bit?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


Removed file: http://bugs.python.org/file27243/posix_path_converter_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


Removed file: http://bugs.python.org/file27235/posix_path_converter.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> Patch looks fine, except please fix 80 columns.

Patch updated. Long lines in tests fixed.

--
Added file: http://bugs.python.org/file27247/posix_path_converter_3.patch

___
Python tracker 

___diff -r 59a2807872d5 Lib/test/test_posix.py
--- a/Lib/test/test_posix.pyFri Sep 21 17:29:20 2012 +0300
+++ b/Lib/test/test_posix.pyFri Sep 21 20:07:09 2012 +0300
@@ -358,12 +358,28 @@
 try:
 self.assertTrue(posix.fstat(fp.fileno()))
 self.assertTrue(posix.stat(fp.fileno()))
+
+self.assertRaisesRegex(TypeError,
+'should be string, bytes or integer, not',
+posix.stat, float(fp.fileno()))
 finally:
 fp.close()
 
 def test_stat(self):
 if hasattr(posix, 'stat'):
 self.assertTrue(posix.stat(support.TESTFN))
+self.assertTrue(posix.stat(os.fsencode(support.TESTFN)))
+self.assertTrue(posix.stat(bytearray(os.fsencode(support.TESTFN
+
+self.assertRaisesRegex(TypeError,
+'can\'t specify None for path argument',
+posix.stat, None)
+self.assertRaisesRegex(TypeError,
+'should be string, bytes or integer, not',
+posix.stat, list(support.TESTFN))
+self.assertRaisesRegex(TypeError,
+'should be string, bytes or integer, not',
+posix.stat, list(os.fsencode(support.TESTFN)))
 
 @unittest.skipUnless(hasattr(posix, 'mkfifo'), "don't have mkfifo()")
 def test_mkfifo(self):
@@ -721,6 +737,14 @@
 s1 = posix.stat(support.TESTFN)
 s2 = posix.stat(support.TESTFN, dir_fd=f)
 self.assertEqual(s1, s2)
+s2 = posix.stat(support.TESTFN, dir_fd=None)
+self.assertEqual(s1, s2)
+self.assertRaisesRegex(TypeError, 'should be integer, not',
+posix.stat, support.TESTFN, dir_fd=posix.getcwd())
+self.assertRaisesRegex(TypeError, 'should be integer, not',
+posix.stat, support.TESTFN, dir_fd=float(f))
+self.assertRaises(OverflowError,
+posix.stat, support.TESTFN, dir_fd=10**20)
 finally:
 posix.close(f)
 
diff -r 59a2807872d5 Modules/posixmodule.c
--- a/Modules/posixmodule.c Fri Sep 21 17:29:20 2012 +0300
+++ b/Modules/posixmodule.c Fri Sep 21 20:07:09 2012 +0300
@@ -427,26 +427,24 @@
 #endif
 
 static int
-_fd_converter(PyObject *o, int *p, int default_value) {
-long long_value;
-if (o == Py_None) {
-*p = default_value;
-return 1;
-}
-if (PyFloat_Check(o)) {
-PyErr_SetString(PyExc_TypeError,
-"integer argument expected, got float" );
+_fd_converter(PyObject *o, int *p, const char *allowed)
+{
+int overflow;
+long long_value = PyLong_AsLongAndOverflow(o, &overflow);
+if (PyFloat_Check(o) ||
+(long_value == -1 && !overflow && PyErr_Occurred())) {
+PyErr_Clear();
+PyErr_Format(PyExc_TypeError,
+"argument should be %s, not %.200s",
+allowed, Py_TYPE(o)->tp_name);
 return 0;
 }
-long_value = PyLong_AsLong(o);
-if (long_value == -1 && PyErr_Occurred())
-return 0;
-if (long_value > INT_MAX) {
+if (overflow > 0 || long_value > INT_MAX) {
 PyErr_SetString(PyExc_OverflowError,
 "signed integer is greater than maximum");
 return 0;
 }
-if (long_value < INT_MIN) {
+if (overflow < 0 || long_value < INT_MIN) {
 PyErr_SetString(PyExc_OverflowError,
 "signed integer is less than minimum");
 return 0;
@@ -456,8 +454,13 @@
 }
 
 static int
-dir_fd_converter(PyObject *o, void *p) {
-return _fd_converter(o, (int *)p, DEFAULT_DIR_FD);
+dir_fd_converter(PyObject *o, void *p)
+{
+if (o == Py_None) {
+*(int *)p = DEFAULT_DIR_FD;
+return 1;
+}
+return _fd_converter(o, (int *)p, "integer");
 }
 
 
@@ -634,17 +637,16 @@
 }
 else {
 PyErr_Clear();
-bytes = PyBytes_FromObject(o);
+if (PyObject_CheckBuffer(o))
+bytes = PyBytes_FromObject(o);
+else
+bytes = NULL;
 if (!bytes) {
 PyErr_Clear();
 if (path->allow_fd) {
 int fd;
-/*
- * note: _fd_converter always permits None.
- * but we've already done our None check.
- * so o cannot be None at this point.
- */
-int result = _fd_converter(o, &fd, -1);
+int result = _fd_converter(o, &fd,
+"string, bytes or intege

[issue15972] wrong error message for os.path.getsize

[Larry Hastings]

Larry Hastings added the comment:

Patch looks fine, except please fix 80 columns.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[John Taylor]

John Taylor added the comment:

OP here.  These error messages are much better.  Thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Patch updated. Added tests.

--
Added file: http://bugs.python.org/file27243/posix_path_converter_2.patch

___
Python tracker 

___diff -r 24136a775943 Lib/test/test_posix.py
--- a/Lib/test/test_posix.pyThu Sep 20 23:49:33 2012 -0400
+++ b/Lib/test/test_posix.pyFri Sep 21 11:47:25 2012 +0300
@@ -358,12 +358,24 @@
 try:
 self.assertTrue(posix.fstat(fp.fileno()))
 self.assertTrue(posix.stat(fp.fileno()))
+
+self.assertRaisesRegex(TypeError, 'should be string, bytes or 
integer, not',
+posix.stat, float(fp.fileno()))
 finally:
 fp.close()
 
 def test_stat(self):
 if hasattr(posix, 'stat'):
 self.assertTrue(posix.stat(support.TESTFN))
+self.assertTrue(posix.stat(os.fsencode(support.TESTFN)))
+self.assertTrue(posix.stat(bytearray(os.fsencode(support.TESTFN
+
+self.assertRaisesRegex(TypeError, 'can\'t specify None for path 
argument',
+posix.stat, None)
+self.assertRaisesRegex(TypeError, 'should be string, bytes or 
integer, not',
+posix.stat, list(support.TESTFN))
+self.assertRaisesRegex(TypeError, 'should be string, bytes or 
integer, not',
+posix.stat, list(os.fsencode(support.TESTFN)))
 
 @unittest.skipUnless(hasattr(posix, 'mkfifo'), "don't have mkfifo()")
 def test_mkfifo(self):
@@ -721,6 +733,13 @@
 s1 = posix.stat(support.TESTFN)
 s2 = posix.stat(support.TESTFN, dir_fd=f)
 self.assertEqual(s1, s2)
+s2 = posix.stat(support.TESTFN, dir_fd=None)
+self.assertEqual(s1, s2)
+self.assertRaisesRegex(TypeError, 'should be integer, not',
+posix.stat, support.TESTFN, dir_fd=posix.getcwd())
+self.assertRaisesRegex(TypeError, 'should be integer, not',
+posix.stat, support.TESTFN, dir_fd=float(f))
+self.assertRaises(OverflowError, posix.stat, support.TESTFN, 
dir_fd=10**20)
 finally:
 posix.close(f)
 
diff -r 24136a775943 Modules/posixmodule.c
--- a/Modules/posixmodule.c Thu Sep 20 23:49:33 2012 -0400
+++ b/Modules/posixmodule.c Fri Sep 21 11:47:25 2012 +0300
@@ -427,26 +427,24 @@
 #endif
 
 static int
-_fd_converter(PyObject *o, int *p, int default_value) {
-long long_value;
-if (o == Py_None) {
-*p = default_value;
-return 1;
-}
-if (PyFloat_Check(o)) {
-PyErr_SetString(PyExc_TypeError,
-"integer argument expected, got float" );
+_fd_converter(PyObject *o, int *p, const char *allowed)
+{
+int overflow;
+long long_value = PyLong_AsLongAndOverflow(o, &overflow);
+if (PyFloat_Check(o) ||
+(long_value == -1 && !overflow && PyErr_Occurred())) {
+PyErr_Clear();
+PyErr_Format(PyExc_TypeError,
+"argument should be %s, not %.200s",
+allowed, Py_TYPE(o)->tp_name);
 return 0;
 }
-long_value = PyLong_AsLong(o);
-if (long_value == -1 && PyErr_Occurred())
-return 0;
-if (long_value > INT_MAX) {
+if (overflow > 0 || long_value > INT_MAX) {
 PyErr_SetString(PyExc_OverflowError,
 "signed integer is greater than maximum");
 return 0;
 }
-if (long_value < INT_MIN) {
+if (overflow < 0 || long_value < INT_MIN) {
 PyErr_SetString(PyExc_OverflowError,
 "signed integer is less than minimum");
 return 0;
@@ -456,8 +454,13 @@
 }
 
 static int
-dir_fd_converter(PyObject *o, void *p) {
-return _fd_converter(o, (int *)p, DEFAULT_DIR_FD);
+dir_fd_converter(PyObject *o, void *p)
+{
+if (o == Py_None) {
+*(int *)p = DEFAULT_DIR_FD;
+return 1;
+}
+return _fd_converter(o, (int *)p, "integer");
 }
 
 
@@ -634,17 +637,16 @@
 }
 else {
 PyErr_Clear();
-bytes = PyBytes_FromObject(o);
+if (PyObject_CheckBuffer(o))
+bytes = PyBytes_FromObject(o);
+else
+bytes = NULL;
 if (!bytes) {
 PyErr_Clear();
 if (path->allow_fd) {
 int fd;
-/*
- * note: _fd_converter always permits None.
- * but we've already done our None check.
- * so o cannot be None at this point.
- */
-int result = _fd_converter(o, &fd, -1);
+int result = _fd_converter(o, &fd,
+"string, bytes or integer");
 if (result) {
 path->wide = NULL;
 path->narrow = NULL;
@@ -705,15 +707,17 @@
 }
 
 static int
-dir_fd_unav

[issue15972] wrong error message for os.path.getsize

[Larry Hastings]

Larry Hastings added the comment:

Ah!  It seems Python is anti-Oxford Comma.  Carry on!  ;-)

Wouldn't test_os be the natural place?  I don't understand why you're having 
difficulty finding a suitable place.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> But please add regression tests checking
> that the error message is what we want.

Immediately as soon as I find a suitable place for this test. Should be 
somewhere tests for bytes/unicode filenames.

> I'd personally prefer the Oxford Comma there ("string, bytes, or integer").
>  But I don't know if there is a style preference one way or the other in
> Python error messages.

"Invalid whence (%i, should be 0, 1 or 2)"
"expect bytes or str of length 1, or int, "
"argument should be bytes, buffer or ASCII string, "
"coercing to str: need bytes, bytearray or buffer-like object, %.80s found"
"character mapping must return integer, bytes or None, not %.400s"

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Larry Hastings]

Larry Hastings added the comment:

Patch looks like it'll work fine.  But please add regression tests checking 
that the error message is what we want.

Are the new error messages okay with the OP?  It looks like now it'll throw 
TypeError("argument must be string, bytes or integer, not list").

v
I'd personally prefer the Oxford Comma there ("string, bytes, or integer").  
But I don't know if there is a style preference one way or the other in Python 
error messages.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Here is a patch.

Are there any tests for string and bytes arguments as filenames? I will add 
float and list there.

--
keywords: +patch
Added file: http://bugs.python.org/file27235/posix_path_converter.patch

___
Python tracker 

___diff -r ddec854843f1 Modules/posixmodule.c
--- a/Modules/posixmodule.c Thu Sep 20 09:47:41 2012 +0300
+++ b/Modules/posixmodule.c Thu Sep 20 20:50:29 2012 +0300
@@ -427,26 +427,24 @@
 #endif
 
 static int
-_fd_converter(PyObject *o, int *p, int default_value) {
-long long_value;
-if (o == Py_None) {
-*p = default_value;
-return 1;
-}
-if (PyFloat_Check(o)) {
-PyErr_SetString(PyExc_TypeError,
-"integer argument expected, got float" );
+_fd_converter(PyObject *o, int *p, const char *allowed)
+{
+int overflow;
+long long_value = PyLong_AsLongAndOverflow(o, &overflow);
+if (PyFloat_Check(o) ||
+(long_value == -1 && !overflow && PyErr_Occurred())) {
+PyErr_Clear();
+PyErr_Format(PyExc_TypeError,
+"argument must be %s, not %.200s",
+allowed, Py_TYPE(o)->tp_name);
 return 0;
 }
-long_value = PyLong_AsLong(o);
-if (long_value == -1 && PyErr_Occurred())
-return 0;
-if (long_value > INT_MAX) {
+if (overflow > 0 || long_value > INT_MAX) {
 PyErr_SetString(PyExc_OverflowError,
 "signed integer is greater than maximum");
 return 0;
 }
-if (long_value < INT_MIN) {
+if (overflow < 0 || long_value < INT_MIN) {
 PyErr_SetString(PyExc_OverflowError,
 "signed integer is less than minimum");
 return 0;
@@ -456,8 +454,13 @@
 }
 
 static int
-dir_fd_converter(PyObject *o, void *p) {
-return _fd_converter(o, (int *)p, DEFAULT_DIR_FD);
+dir_fd_converter(PyObject *o, void *p)
+{
+if (o == Py_None) {
+*(int *)p = DEFAULT_DIR_FD;
+return 1;
+}
+return _fd_converter(o, (int *)p, "integer");
 }
 
 
@@ -634,17 +637,16 @@
 }
 else {
 PyErr_Clear();
-bytes = PyBytes_FromObject(o);
+if (PyObject_CheckBuffer(o))
+bytes = PyBytes_FromObject(o);
+else
+bytes = NULL;
 if (!bytes) {
 PyErr_Clear();
 if (path->allow_fd) {
 int fd;
-/*
- * note: _fd_converter always permits None.
- * but we've already done our None check.
- * so o cannot be None at this point.
- */
-int result = _fd_converter(o, &fd, -1);
+int result = _fd_converter(o, &fd,
+   "string, bytes or integer");
 if (result) {
 path->wide = NULL;
 path->narrow = NULL;
@@ -705,15 +707,17 @@
 }
 
 static int
-dir_fd_unavailable(PyObject *o, void *p) {
-int *dir_fd = (int *)p;
-int return_value = _fd_converter(o, dir_fd, DEFAULT_DIR_FD);
-if (!return_value)
+dir_fd_unavailable(PyObject *o, void *p)
+{
+int dir_fd;
+if (!dir_fd_converter(o, &dir_fd))
 return 0;
-if (*dir_fd == DEFAULT_DIR_FD)
-return 1;
-argument_unavailable_error(NULL, "dir_fd");
-return 0;
+if (dir_fd != DEFAULT_DIR_FD) {
+argument_unavailable_error(NULL, "dir_fd");
+return 0;
+}
+*(int *)p = dir_fd;
+return 1;
 }
 
 static int
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[STINNER Victor]

STINNER Victor added the comment:

Set the priority to release blocker until it is decided if this issue is a 
regression, or a new feature :-)

--
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[STINNER Victor]

STINNER Victor added the comment:

Functions of the os module uses PyUnicode_FSConverter() function (which uses 
PyBytes_Check() on bytes) in Python 3.2, whereas PyBytes_FromObject() is used 
in Python 3.3. Related change:

changeset:   77597:27f9c26fdd8b
user:Larry Hastings 
date:Fri Jun 22 16:30:09 2012 -0700
files:   Doc/library/os.rst Lib/os.py Lib/shutil.py Lib/test/support.py 
Lib/test/test_os.py Lib/test/test_posix.py Lib/test/test_shutil.py Misc/NEWS 
Modules/posixmodule.c
description:
Issue #14626: Large refactoring of functions / parameters in the os module.
Many functions now support "dir_fd" and "follow_symlinks" parameters;
some also support accepting an open file descriptor in place of of a path
string.  Added os.support_* collections as LBYL helpers.  Removed many
functions only previously seen in 3.3 alpha releases (often starting with
"f" or "l", or ending with "at").  Originally suggested by Serhiy Storchaka;
implemented by Larry Hastings.

--
nosy: +larry, storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[STINNER Victor]

STINNER Victor added the comment:

It looks like os.stat() and os.path.getsize() converts the list into a byte 
string. It does something like:

>>> x=[]; y=bytes(x); print(y.decode("ascii"))

>>> x=[65, 66, 67]; y=bytes(x); print(y.decode("ascii"))
ABC
>>> x=[None]; y=bytes(x); print(y.decode("ascii"))
Traceback (most recent call last):
  File "", line 1, in 
TypeError: 'NoneType' object cannot be interpreted as an integer

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[Christian Heimes]

Christian Heimes added the comment:

Linux:

>>> os.stat([])
Traceback (most recent call last):
  File "", line 1, in 
FileNotFoundError: [Errno 2] No such file or directory: ''
[60996 refs]
>>> os.stat([None])
Traceback (most recent call last):
  File "", line 1, in 
TypeError: an integer is required
[60993 refs]

--
keywords: +3.3regression
nosy: +christian.heimes

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15972] wrong error message for os.path.getsize

[John Taylor]

New submission from John Taylor:

import os.path
a = [ r'c:\Windows\notepad.exe' ]
print( os.path.getsize(a) )

Under Python 3.2.3, this error message is returned:
  File "c:\python32\lib\genericpath.py", line 49, in getsize
return os.stat(filename).st_size
TypeError: Can't convert 'list' object to str implicitly


Under Python 3.3.0rc2, this error message is returned:
  File "c:\Python33\lib\genericpath.py", line 49, in getsize
return os.stat(filename).st_size
TypeError: an integer is required


I feel like the 3.2.3 behavior is more accurate and would like to propose that 
the 3.3 error message says something about a list instead of an integer.

--
components: Extension Modules
messages: 170726
nosy: jftuga
priority: normal
severity: normal
status: open
title: wrong error message for os.path.getsize
type: behavior
versions: Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com