Repository navigation
files, opened in unicode (text): write() returns symbols count, but seek() expect offset in bytes #70036
Description
| BPO | 25849 |
|---|---|
| Nosy | @pitrou, @vstinner, @ezio-melotti, @bitdancer, @socketpair, @vadmium |
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 2017-11-09.17:58:10.838>
created_at = <Date 2015-12-12.20:19:49.560>
labels = ['invalid', 'library', 'expert-unicode', 'expert-IO']
title = 'files, opened in unicode (text): write() returns symbols count, but seek() expect offset in bytes'
updated_at = <Date 2017-11-09.17:58:10.837>
user = 'https://github.com/socketpair'bugs.python.org fields:
activity = <Date 2017-11-09.17:58:10.837>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-11-09.17:58:10.838>
closer = 'serhiy.storchaka'
components = ['Library (Lib)', 'Unicode', 'IO']
creation = <Date 2015-12-12.20:19:49.560>
creator = 'socketpair'
dependencies = []
files = []
hgrepos = []
issue_num = 25849
keywords = []
message_count = 15.0
messages = ['256291', '256293', '256324', '256329', '256415', '256416', '256417', '256418', '256433', '256440', '256447', '256448', '256551', '256559', '256563']
nosy_count = 6.0
nosy_names = ['pitrou', 'vstinner', 'ezio.melotti', 'r.david.murray', 'socketpair', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue25849'
versions = ['Python 3.3', 'Python 3.4', 'Python 3.5', 'Python 3.6']
Activity
socketpair commentedon Dec 12, 2015
It seems, that we should deprecate .seek() on files, opened in text mode.
Since it is not possible to seek to position between symbols. Yes, it is possible to decode UTF-8 (or other charset) starting from beginning of the file and count symbols, but it is EXTREMELY SLOW, and is not what user expect. If so, seeking from end of file back to begin may be implemented in even more hard and error-prone way.
Moreover, I consider that we should disallow seek in text files except seek() to begin of the file (position 0) or end of file (seek(0, SEEK_END)).
Seel also bpo-25190 bpo-25190 about something related for that.
bitdancer commentedon Dec 12, 2015
As mentioned in those issues, currently the peek/seek token is a black box. That doesn't mean it isn't useful. Those issues are talking about potential ways to make it more useful, so any discussion should occur there.
socketpair commentedon Dec 13, 2015
https://docs.python.org/3.5/library/io.html?highlight=stringio#id3 :
Also, TextIOWrapper.tell() and TextIOWrapper.seek() are both quite slow due to the reconstruction algorithm used.
What is reconstruction algorightm ? Experiments show, that seek() and tell() returns values of count of bytes (not letters).
So, please reopen. Issue is still here.
bitdancer commentedon Dec 13, 2015
I'm still not seeing a bug.
If you have a performance enhancement or functional enhancement you'd like us to consider, please attach a patch, with benchmark results.
Since you say "are quite slow because of the reconstruction algorithm", what makes you say this? I'd think the "algorithm" was just using the underlying bytes tell/seek value, which then becomes a black box token because it does not have a one to one releationship to the character count.
socketpair commentedon Dec 14, 2015
First, it seems that there are no real "reconstruction algorithm" at all. Seek is allowed to point to any byte position, even to place "inside" characters for multibyte encodings, such as UTF-8.
Second, about performance: I talk about implementation mentioned in first message. If it is not used (and will not be used), we may forget about that sentence.
Next, once again:
I consider it is a bug in allowing to seek to invalid byte offsets for text files. Since we cannot easily calculate what offset will be valid (for example, seek past the end of file, or places inside character), just disallow seek. In real applications, no one will seek/peek to places other than
so this seeks/peeks must be allowed.
This is applicable only to variable multibyte encodings (such as UTF-8).
socketpair commentedon Dec 14, 2015
s/peek/tell/
socketpair commentedon Dec 14, 2015
Also, can you provide the case, where such random seeks can be used on text files? It would be programmer error to seek to places other I mention. Does not it ?
bitdancer commentedon Dec 14, 2015
I think you haven't quite gotten what "opaque token" means in this context. The way you use tell/seek with text files is: you have read to some certain point in the file. You call 'tell' and get back an opqaue token. Later you can call seek with that token to get back to the place in the file that you "bookmarked". It will never be between characters, because tell won't return such a poitner. If you decide to call seek with something (other than 0) that you didn't get from tell, then you are on your own.
vadmium commentedon Dec 15, 2015
You might be right about the “reconstruction algorithm”. This text was added in revision 0bba533c0959; maybe Antoine can comment whether we should clarify or remove it.
I think the text added for TextIOBase.seek() in revision 03e61104f7a2 (bpo-12922) is closer to the truth. Though I would probably drop the bit about tell() not usually returning a byte position; for many codecs it does seem to.
This illustrates the only four cases of seeking I understand are allowed for text streams:
If the “slow reconstruction algorithm” was clarified or removed, and the documentation explained that you cannot seek to arbitrary characters without having previously called tell(), would that work?
vadmium commentedon Dec 15, 2015
I’m starting to understand that there might be a “reconstruction algorithm” needed. When reading, TextIOWrapper buffers decoded characters. If you call tell() and there is unread but decoded data, it is not enough to return the incremental decoder state. You have to handle the unread buffered data as well. Looking at the _pyio tell() implementation, it tries to wind the decoder backwards to minimize the state.
pitrou commentedon Dec 15, 2015
I don't understand what the complaint is. If you think seek()/tell() are not useful, just don't use them.
vstinner commentedon Dec 15, 2015
I wrote this algorithm, or I helpd to write it, I don't recall.
The problem is readahead: TextIOWrapper read more bytes than requested for performances. But when tell() is called, the user expects to get the current file position, not the "read ahead" file position. So we have to go backward. Problem: TextIOWrapper uses text (Unicode) whereas all files are bytes on the disk. We need to compute the size of the readahead buffer in bytes from a buffer in characters.
The bad performances comes from multibyte codecs which requires heuristic to first guess the number of bytes and then really encode back bytes to find the exact size.
See _pyio.TextIOWrapper.tell() for the Python implementation.
# Fast search for an acceptable start point, close to our
# current pos.
# Rationale: calling decoder.decode() has a large overhead
# regardless of chunk size; we want the number of such calls to
# be O(1) in most situations (common decoders, non-crazy input).
# Actually, it will be exactly 1 for fixed-size codecs (all
# 8-bit codecs, also UTF-16 and UTF-32).
(Incomplete) history of the Python implementation of the tell() method:
socketpair commentedon Dec 16, 2015
Well, 03e61104f7a2 adds good description, why not to enforce checks instead of saying that some values are unsupported ?
Also, idea in returning special object instance from tell(), this object should incapsulate byte offset. And allow for the seek() either such objects or zero.
socketpair commentedon Dec 16, 2015
Seems this object should also incapsulate weakref to opened file... (to prevent using it on another text file)
One bad thing: seeking to the end of file is not safe:
If someone opens logfile, last byte in that text file may belong to middle of multibyte sequence. This really may happen, since Linux (libc) writes files page-by-page (if not line-buffered), so if page boundary is inside character, this bad thing may happen. Real case - is implementing something like
tail -fin python (to, say, monitor log files).So, seeking to the end of file is also wrong, and should be disallowed. If so, how to append text to text files ? very upset :( :( I don't know decision of that problem.
vadmium commentedon Dec 16, 2015
I think changing the TextIOBase API would be hard to do if you want to keep compatibility with existing code. I agree that encoding the position to a number and back seems like a bad design, but I doubt it is worth changing it at this point.