[Patches] [Patch #101512] Support parsing of already opened file objects

[noreply@sourceforge.net]

Patch #101512 has been updated. 

Project: 
Category: XML
Status: Open
Summary: Support parsing of already opened file objects

Follow-Ups:

Date: 2000-Sep-14 11:15
By: jhylton

Comment:
I don't particularly care for this interface change.  It appears that the parser will close the file if passed as an argument.  Since the parser did not open the file, it should not close it.

It seems to me that this feature should be a separate method.

Also, there seem to be a lot of style problems with this code.  There is inconsistent use of whitespace around ()s and =s.  It should be consistent above all!  And it should use the approved Python style.

-------------------------------------------------------

Date: 2000-Sep-14 12:21
By: prescod

Comment:
This isn't an interface change, it just makes the interface work the way it says it does. If fixes a FIXME.

A separate method is a good idea but it would have to be agreed upon by the xml-sig (or at least implemented by Lars, the main SAX maintainer). I see that as part of the larger refactoring that Lars is doing.

I'm not clear what line of code closes the file. It looks to me like it just goes out of scope still open.
-------------------------------------------------------

Date: 2000-Sep-14 14:08
By: fdrake

Comment:
I'm hesitant to approve this myself; I don't like the overloading.  (Though I agree that this simply brings the implementation into line with the documentation, such as it is.)

The Java equivelent uses an overloaded parse() method, but Java provides direct support for this in the language definition.  I think the Python SAX2 definition should provide separate methods.

Let's wait for Lars to weigh in on the API issues.
-------------------------------------------------------

Date: 2000-Sep-15 04:03
By: gvanrossum

Comment:
I would approve this if it took care of Jeremy's concern: if the file is opened by the caller, the callee should not close it.

Given that Java overloads parse(), the Python approach of testing for a file is fine; I don't think it's necessary to dd a new method.
-------------------------------------------------------

Date: 2000-Sep-15 06:53
By: prescod

Comment:
I believe Jeremy to be wrong. If you do a "grep" in the xml/sig directory you'll find that files are never closed. They just go out of scope. If we opened the file then it will close when it goes out of scope. If the caller opened the file, it will close when they throw away their last reference to it. The wonders of reference counting!
-------------------------------------------------------

Date: 2000-Sep-15 06:56
By: jhylton

Comment:
It sounds like the code has the opposite sort of bug, then.  It opens file and does not explicitly close them.  The code should not depend on the reference counting mechanism to close files.

-------------------------------------------------------

Date: 2000-Sep-15 07:29
By: prescod

Comment:
The word bug is too strong when all the module does is depend on documented Python features. Allowing auto-closing of files is one of the major arguments for Python's reference counting (according to the Python FAQ) and an advantage of Python over Java et al. If you do explicit closing you reintroduce the same lifetime issues that make pointer handling such a hassle in C++.

In this particular case, the lifetime of the file object is pretty easy to track (barring funny stuff) so adding a close would be relatively easy and would make portability to JPython easier. I would like to hear what the BeOpen concensus is so that I can add it to my personal style guide.
-------------------------------------------------------

Date: 2000-Sep-15 07:52
By: gvanrossum

Comment:
BeOpen doesn't enter into it. :-)  PythonLabs is what counts.

Consensus is had has been as Jeremy says: iff you open a file, you should close it.

Note: adding close() should be a separate patch.
Fred: you can accept this one once the close() patch is in.
-------------------------------------------------------

-------------------------------------------------------
For more info, visit:

http://sourceforge.net/patch/?func=detailpatch&patch_id=101512&group_id=5470