Issue1650802

classification
Title: _csv module
Type: rfe Severity: normal
Components: Library Versions: 2.5alpha1
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: pjenvey Nosy List: cgroves, dkuhlman, pdrummond, pjenvey
Priority: high Keywords: patch

Created on 2007-02-02.17:08:47 by pdrummond, last changed 2008-08-06.02:48:57 by pjenvey.

Files
File name Uploaded Description Edit Remove
_csv.patch pdrummond, 2007-02-02.17:08:48
csv_svn.zip dkuhlman, 2008-06-10.18:11:46 Zip file containing CSV implementation files
Messages
msg2637 (view) Author: Paul Drummond (pdrummond) Date: 2007-02-02.17:08:47
a patch for the _csv module in 2.3.  I have included a version of test_csv.py with the "gc" module dependencies commented out.  One test fails - test_delimiters - which as far as I can tell is an issue with csv.py rather than the _csv.java (see http://sourceforge.net/mailarchive/forum.php?thread_id=31550745&forum_id=5587 for more details).
msg2638 (view) Author: Charlie Groves (cgroves) Date: 2007-02-12.06:00:14
This looks like a good start, but there are several issues with the integration with Jython that need to be fixed.

First, the Dialect class is a new-style class, but it looks like you've written it by hand.  All of the stuff in typeSetup should be generated by src/templates/gexpose.py and a corresponding .expose file.  You can look at PyString, PyList, PyDictionary or any of the other new style classes in core to see how things work in general.  Using the templating system will also help with the __delattr__, __findattr__ and __setattr__ implementations you have in Dialect.  Rather than implementing those as part of the class, the pieces that need custom behavior for those methods can be handled by the descriptors generated by the templates.  Unfortunately, I can't do much better than pointing you at the existing templates and their results as it's not documented at all.  

As a starting point, I'd suggest creating a new csv package in modules like the ones for sre, time and so on.  Move the other two inner classes to the new package as top level classes and redo Dialect as a generated class with a new dialect.expose in src/templates output to there.  If Dialect needs to be subclassed, it'll need a dialect.derived as well.  You can see the results of the *derived in all of the *Derived.java classes for the newstyle classes.  They just allow methods to be overridden by subclasses of builtin types.  After that, _csv.java will be mostly a shell referencing the functionality in the actual object classes.  

A better method for handling all of the optional arguments to the constructor is to use the getPyObject(int pos, PyObject default) and include the default values you're currently including earlier in the constructor as PyObjects passed to the getPyObject method as default.  Then use Py.py2<type> to turn it into the type you expect.  As a bonus, this correctly handles checking that the passed in kwarg is of the right type and allows you to pass in a message to be reported in the TypeError if it's inappropriate.

Feel free to ask any general questions about the new style stuff on jython-dev.  They definitely need more exposure and documentation.  Actually, any general questions about internal jython usage would be good for the jython-dev list.  I'd only comment on here if it's specific to the csv api.  
msg3270 (view) Author: Dave Kuhlman (dkuhlman) Date: 2008-06-10.18:11:40
Update to the CSV module.

This version passes all but 4 regression tests.  Those 4 failures all
seem to be related to the inability to create instances of the (java)
Dialog class.
msg3405 (view) Author: Philip Jenvey (pjenvey) Date: 2008-08-06.02:48:56
I cleaned up everything to our coding standards (except for camel casing 
names), fixed the remaining 2.3 test failures and applied the 2.5 changes. 
committed in r5090

Thanks very much Dave and Paul!
History
Date User Action Args
2008-08-06 02:48:57pjenveysetstatus: open -> closed
type: rfe
resolution: fixed
messages: + msg3405
2008-08-02 21:05:30pjenveysetassignee: pjenvey
nosy: + pjenvey
2008-06-13 05:40:04pjenveysetpriority: normal -> high
2008-06-10 18:11:54dkuhlmansetfiles: + csv_svn.zip
nosy: + dkuhlman
messages: + msg3270
components: + Library, - None
versions: + 2.5alpha1
2007-02-02 17:08:47pdrummondcreate