Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strengthen 2.7 io types warning #56643

Closed
terryjreedy opened this issue Jun 28, 2011 · 10 comments
Closed

Strengthen 2.7 io types warning #56643

terryjreedy opened this issue Jun 28, 2011 · 10 comments
Labels
docs Documentation in the Doc dir easy

Comments

@terryjreedy
Copy link
Member

BPO 12434
Nosy @terryjreedy, @pitrou, @benjaminp, @merwok

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2011-07-22.11:41:20.256>
created_at = <Date 2011-06-28.19:22:57.803>
labels = ['easy', 'docs']
title = 'Strengthen 2.7 io types warning'
updated_at = <Date 2011-07-22.11:41:20.255>
user = 'https://github.com/terryjreedy'

bugs.python.org fields:

activity = <Date 2011-07-22.11:41:20.255>
actor = 'eli.bendersky'
assignee = 'docs@python'
closed = True
closed_date = <Date 2011-07-22.11:41:20.256>
closer = 'eli.bendersky'
components = ['Documentation']
creation = <Date 2011-06-28.19:22:57.803>
creator = 'terry.reedy'
dependencies = []
files = []
hgrepos = []
issue_num = 12434
keywords = ['easy']
message_count = 10.0
messages = ['139374', '140138', '140171', '140173', '140482', '140492', '140603', '140649', '140650', '140871']
nosy_count = 8.0
nosy_names = ['terry.reedy', 'pitrou', 'benjamin.peterson', 'stutzbach', 'eric.araujo', 'eli.bendersky', 'docs@python', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'needs patch'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue12434'
versions = []

@terryjreedy
Copy link
Member Author

Trying 3.2 code with 2.7, I got this (greatly simplified):

from __future__ import print_function
from io import StringIO
print('hello world', file=StringIO())
Traceback...
TypeError: string argument expected, got 'str'
(StringIO.StringIO works fine, of course.)

This was initially confusing. Suggestion: after

"Note Since this module has been designed primarily for Python 3.x, you have to be aware that all uses of “bytes” in this document refer to the str type (of which bytes is an alias), and all uses of “text” refer to the unicode type. "
add
'String' in exception messages may also mean the unicode type."

@terryjreedy terryjreedy added docs Documentation in the Doc dir easy labels Jun 28, 2011
@merwok
Copy link
Member

merwok commented Jul 11, 2011

Do you think the docstrings and error messages should be improved as well as the docs?

@terryjreedy
Copy link
Member Author

My original suggestion is a minimal change suggestion. I do not have much opinion on what the maximum change 'should' be.

It would obviously be nice from a user viewpoint if the error message were backported to say "unicode argument expected, got 'str'". Then the note change might not be needed. But I do not know if the particular message is an accident or part of a policy of not fully backporting the code (to aid future patching?), or whether there are other messages that would need the same treatment.

The module docstring shown by help(io) does not have the terminology note to be augmented.

@elibendersky
Copy link
Mannequin

elibendersky mannequin commented Jul 12, 2011

In my opinion, it's the error messages and docstrings that should be changed, not the documentation. This module was introduced in 2.6 and moves on to 2.7, and there's no reason to have it throw confusing errors for the sake of easier back-patching from 3.x

However, when I run this example on 2.6, I get:

TypeError: can't write str to text stream

Which (arguably) makes sense, since the docs explicitly say that "Text I/O classes work with unicode data."

@elibendersky
Copy link
Mannequin

elibendersky mannequin commented Jul 16, 2011

The difference between 2.6 and 2.7 stems from the rewrite of the IO library in C that was made for 2.7

The error Terry sees gets thrown here (in Modules/_io/stringio.c):

    if (!PyUnicode_Check(obj)) {
        PyErr_Format(PyExc_TypeError, "string argument expected, got '%s'",
                     Py_TYPE(obj)->tp_name);
        return NULL;
    }

Therefore, I propose to change this error message to:

"unicode argument expected, got '%s'"

as Terry suggested.

Adding Antoine, Benjamin and Daniel (listed as experts on IO) to nosy.

Is there an objection to making this change in the error message?

@pitrou
Copy link
Member

pitrou commented Jul 16, 2011

The error Terry sees gets thrown here (in Modules/_io/stringio.c):

if (!PyUnicode_Check(obj)) {
    PyErr_Format(PyExc_TypeError, "string argument expected, got '%s'",
                 Py_TYPE(obj)-\>tp_name);
    return NULL;
}

Therefore, I propose to change this error message to:

"unicode argument expected, got '%s'"

as Terry suggested.

Adding Antoine, Benjamin and Daniel (listed as experts on IO) to nosy.

Is there an objection to making this change in the error message?

No, the proposal is right.

@stutzbach
Copy link
Mannequin

stutzbach mannequin commented Jul 18, 2011

On Sat, Jul 16, 2011 at 2:04 AM, Eli Bendersky <report@bugs.python.org>wrote:

Therefore, I propose to change this error message to:
"unicode argument expected, got '%s'"
as Terry suggested.

Sounds good to me.

@elibendersky
Copy link
Mannequin

elibendersky mannequin commented Jul 19, 2011

> Therefore, I propose to change this error message to:
> "unicode argument expected, got '%s'"
> as Terry suggested.
>

Sounds good to me.

Terry, what are your thoughts?
Can I commit the fix?

@terryjreedy
Copy link
Member Author

Yes, that would be great. It is better than my initial suggestion.

@python-dev
Copy link
Mannequin

python-dev mannequin commented Jul 22, 2011

New changeset 0752215f9f91 by Eli Bendersky in branch '2.7':
Issue bpo-12434: make StringIO.write error message consistent with Python 2.7 nomenclature
http://hg.python.org/cpython/rev/0752215f9f91

@elibendersky elibendersky mannequin closed this as completed Jul 22, 2011
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir easy
Projects
None yet
Development

No branches or pull requests

3 participants