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

error message of ord is not intuitive #71195

Closed
zhangyangyu opened this issue May 12, 2016 · 9 comments
Closed

error message of ord is not intuitive #71195

zhangyangyu opened this issue May 12, 2016 · 9 comments

Comments

@zhangyangyu
Copy link
Member

BPO 27008
Nosy @bitdancer, @serhiy-storchaka, @zhangyangyu

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 2016-05-12.16:11:08.262>
created_at = <Date 2016-05-12.08:04:50.468>
labels = ['invalid']
title = 'error message of ord is not intuitive'
updated_at = <Date 2016-05-12.16:11:08.261>
user = 'https://github.com/zhangyangyu'

bugs.python.org fields:

activity = <Date 2016-05-12.16:11:08.261>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2016-05-12.16:11:08.262>
closer = 'r.david.murray'
components = []
creation = <Date 2016-05-12.08:04:50.468>
creator = 'xiang.zhang'
dependencies = []
files = []
hgrepos = []
issue_num = 27008
keywords = []
message_count = 9.0
messages = ['265376', '265377', '265378', '265380', '265382', '265384', '265401', '265409', '265415']
nosy_count = 3.0
nosy_names = ['r.david.murray', 'serhiy.storchaka', 'xiang.zhang']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue27008'
versions = ['Python 3.6']

@zhangyangyu
Copy link
Member Author

The error message of ord is not that right. It says 'ord() expected string of length 1'. I don't think in Py3.x string can refer to both bytes and unicodes.

@serhiy-storchaka
Copy link
Member

This not the only place where "string" means unicode string or bytes string. I don't think this is large issue.

@zhangyangyu
Copy link
Member Author

If you think it's OK, it's fine. Please close this thread.

@zhangyangyu
Copy link
Member Author

BTW, what do you think about the second TypeError in ord. Shouldn't it be ValueError?

@serhiy-storchaka
Copy link
Member

In any case it doesn't make much sense to change just one separate docstring. If you want to avoid misleading and support consistent wording, you should examine all occurrences of the word "string" in the documentation, docstrings, error messages and comments -- does it mean Unicode string, bytes-like object that supports the buffer protocol (including memoryview), bytes-like object that supports str-like interface (including bytes, bytearray, but excluding memoryview), either Unicode or bytes string? I tried to do this but abandoned the work on half-way. This is too large work.

BTW, what do you think about the second TypeError in ord. Shouldn't it be ValueError?

It could be ValueError. But for compatibility it should stay TypeError. This is not wrong if we consider strings of size 1 as separate type.

@zhangyangyu
Copy link
Member Author

Ohh, you have tried to do that. It must be a large work.

But on the other hand, if this is a too large work, why not solve this case by case? This work is too large to get someone work on it, even you, the most active developer in the community I see have abandoned. And then maybe improving the situation a little bit every time is the only solution.

This is not wrong if we consider strings of size 1 as separate type

This sounds weird. But I can understand the importance of compatibility.

@bitdancer
Copy link
Member

You are right, if it is too big a job to do it all at once, then we can fix them as we find them. Do you want to propose a patch?

However, in this case I think there is arguably not a bug. It looks as though the intent is that ord only support strings (see the documentation). The fact that it supports bytes-like objects is redudant (ord(b'a') == b'a'[0]). I'd call it a bug that it supports bytes-like objects, but we probably kept (and should keep it) it to make it easier to port python2 code to python3.

So, if any change were to be made here, it would probably be to change the error message if and only if the input is not in fact a string, and perhaps even recommend using the indexing syntax.

On the gripping hand, I've never been a fan of the fact that indexing a byte string gets you an integer :)

@zhangyangyu
Copy link
Member Author

I also notice the document. I get surprised when I see the implementation also supports bytes while the doc says one unicode character. But then I tell myself that maybe unicode character also includes bytes? I am not sure about the English description.

But giving your opinion that maybe the bytes supports are not intended now, I think leaving the error message untouched is quite OK. It describes what it is intended to do, same as the doc.

Really glad to have your comments, Serhiy and David.

@bitdancer
Copy link
Member

All right, we'll close this then.

@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
None yet
Projects
None yet
Development

No branches or pull requests

3 participants