Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Python: Bugs

[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper

 

 

Python bugs RSS feed   Index | Next | Previous | View Threaded


report at bugs

May 5, 2012, 9:16 AM

Post #1 of 15 (159 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper

Roundup Robot <devnull [at] psf> added the comment:

New changeset 254cb4f5d0ff by Lars Gustäbel in branch 'default':
Issue #13815: TarFile.extractfile() now returns io.BufferedReader objects.
http://hg.python.org/cpython/rev/254cb4f5d0ff

----------
nosy: +python-dev

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 5, 2012, 9:29 AM

Post #2 of 15 (155 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Lars Gustäbel <lars [at] gustaebel> added the comment:

I did some tarfile spring cleaning: I removed the ExFileObject class completely as it was more or less a leftover from the old days. io.BufferedReader now does the job. So, as a side-effect, I close this issue as fixed.

(BTW, this makes tarfile.py smaller by about 100 lines.)

----------
resolution: -> fixed
stage: patch review -> committed/rejected
status: open -> closed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 8, 2012, 11:16 AM

Post #3 of 15 (156 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Amaury Forgeot d'Arc <amauryfa [at] gmail> added the comment:

I think it would have been better to keep the ExFileObject class, and base it on io.BufferedReader:

class ExFileObject(io.BufferedReader):
def __init__(self, tarfile, tarinfo):
raw = _FileInFile(tarfile.fileobj,
tarinfo.offset_data,
tarinfo.size,
tarinfo.sparse)
io.BufferedReader.__init__(self, raw)

The result is the same of course, but there is no need to special-case the pre-3.3 API.
In addition, _FileInFile could probably inherit from io.RawIOBase.

----------
nosy: +amaury.forgeotdarc

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 8, 2012, 4:48 PM

Post #4 of 15 (149 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

R. David Murray <rdmurray [at] bitdance> added the comment:

Indeed, even though it is not a documented API, our backward compatibility policy pretty much requires that something named ExFileObject still exist, just in case. And in this case it probably should still be the thing returned.

----------
nosy: +r.david.murray
status: closed -> open

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 8, 2012, 11:22 PM

Post #5 of 15 (150 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

Well, if it's not documented, it's technically a private API.

Also, there doesn't seem to be any explicit use of ExFileObject outside of tarfile.py:
http://code.google.com/codesearch#search&q=lang:python+exfileobject

----------
nosy: +pitrou

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 9, 2012, 4:44 AM

Post #6 of 15 (152 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Lars Gustäbel <lars [at] gustaebel> added the comment:

In an earlier draft of my patch, I had kept ExFileObject as a subclass of BufferedReader, but I later decided against it. To use BufferedReader directly is in my opinion the cleaner solution.

I admit that the change is not fully backward compatible. But a user can still write code that works for both 3.3 and the versions before. If he didn't subclass ExFileObject his code doesn't even need a change. If he subclassed ExFileObject, he might have a problem in either case: either the ExFileObject class is missing, or he may be unable to use it the way he did before, because all that's left of it is a stub subclass of BufferedReader.

I am well aware that backward compatibility is most important, but I think it must still be allowed to change internal (and undocumented) APIs every now and then to clean things up a little.
And of course, I did a code search before too, and found no code using ExFileObject. This actually doesn't surprise me, as there is really not much you can do with it.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 9, 2012, 6:33 AM

Post #7 of 15 (150 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

R. David Murray <rdmurray [at] bitdance> added the comment:

Yeah, I know it is technically private. We still tend to keep names around unless there's a good reason to delete them (like using them leads to broken code anyway). The code search is some evidence this deletion would be OK, but why *not* follow Amaury's suggestion?

I'm OK if you reclose this, but I unfortunately I don't think simple cleanliness is a good argument (even though I would like it to be). The other arguments are better :)

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 9, 2012, 6:41 AM

Post #8 of 15 (153 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> Yeah, I know it is technically private. We still tend to keep names
> around unless there's a good reason to delete them (like using them
> leads to broken code anyway). The code search is some evidence this
> deletion would be OK, but why *not* follow Amaury's suggestion?

I don't see the point of maintaining a private API that's proven to be
unused :) It's an unwarranted maintenance burden (though admittedly a
light one here).

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 9, 2012, 7:18 AM

Post #9 of 15 (150 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

R. David Murray <rdmurray [at] bitdance> added the comment:

Code search is not proof, I'm afraid. It is evidence, though, and I thought I indicated I thought it was a good argument in favor of dropping the class.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 9, 2012, 7:40 AM

Post #10 of 15 (148 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> Code search is not proof, I'm afraid. It is evidence, though, and I
> thought I indicated I thought it was a good argument in favor of
> dropping the class.

Yes, sorry for the vocabulary mismatch :-)

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 9, 2012, 8:34 AM

Post #11 of 15 (158 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Amaury Forgeot d'Arc <amauryfa [at] gmail> added the comment:

I came here when I saw this comment in the diff: "# Keep the traditional pre-3.3 API intact". Why keep an internal API intact if we do it partially?

The ExFileObject class above will also simplify the code: simply "return self.fileobject(self, tarinfo)" in all cases.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 10, 2012, 2:44 AM

Post #12 of 15 (155 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Lars Gustäbel <lars [at] gustaebel> added the comment:

Okay, I attached a patch that I hope we can all agree upon. It restores the ExFileObject class as a small subclass of BufferedReader as Amaury suggested. Does the documentation have to be changed, too? It states that an io.BufferedReader object is returned by extractfile() not a subclass thereof.

----------
Added file: http://bugs.python.org/file25516/tarfile-exfileobj.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 10, 2012, 5:46 AM

Post #13 of 15 (149 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

R. David Murray <rdmurray [at] bitdance> added the comment:

I don't think a doc change is needed. An isinstance check based on the docs will succeed, and the rest is an implementation detail, I think.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 14, 2012, 4:20 AM

Post #14 of 15 (133 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Roundup Robot <devnull [at] psf> added the comment:

New changeset ab6496b98ac4 by Lars Gustäbel in branch 'default':
Issue #13815: Resurrect the ExFileObject class.
http://hg.python.org/cpython/rev/ab6496b98ac4

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 14, 2012, 4:22 AM

Post #15 of 15 (135 views)
Permalink
[issue13815] tarfile.ExFileObject can't be wrapped using io.TextIOWrapper [In reply to]

Lars Gustäbel <lars [at] gustaebel> added the comment:

Okay, I close this issue now, as I think the problems are now resolved.

----------
status: open -> closed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue13815>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Python bugs RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.