From: William S F. <ws...@fu...> - 2006-06-21 19:30:21
|
Joseph Wang wrote: > R-SWIG is at the point where it now compiles all of the C++ unit tests, and > has support for all of the functionality that I know of that was in Duncan > Temple Lang's original implementation (including copyToR). There are still > some work do to which includes removing all of the redundant code and > reimplementing function overloading using the SWIG dispatch mechanism instead > of S4 methods. > > But I'd like to have the code under version control before undertaking these > changes. The R-SWIG is part of my general research on Shanghai derivatives > so I will be committed to supporting the module for an extended time. My > sourceforge ID is drjoe. > > Comments? > > > Sounds good. Can you please post a patch of what your initial commit will be so we can review it first. Thanks William |
From: William S F. <ws...@fu...> - 2006-06-22 21:31:31
|
Hi Joseph, Looks good, but I have some points which need addressing before you check any of this in. I'll set up your cvs access when I get a moment either tonight or tomorrow. Can you add in a runme.R to the class and simple examples please which provide similar output to the other language's runtime tests? I see you have a bunch of .i files in the test-suite. These duplicate tests we already have. The idea with the test-suite is to use the common .i tests. Language specific .i files are for language specific features, so please use the main .i tests. Eg inout.i and output.i can be replaced with the the more comprehensive tests in li_typemaps.i. Note that for each xxx.i library file there is a li_xxx.i testcase. Replace primitives.i with primitive_types.i. In fact all your .i files look like duplicated tests of our existing test cases, so please replace them. It isn't always easy finding an appropriate test if you are looking at writing runtime tests, so please ask. The only exception I can see is your 'opaque' feature, in which case this should be in something like an opaque_feature.i test. Please follow the normal %feature conventions for your opaque flag feature, see the Customization.html#Customization_feature_flags. Use GetFlag() in r.cxx code for feature:new and feature:immutable and feature:opaque. Please add in the usual copyright notice and format the top of r.cxx the same as all the other language module files. Keep all variables to file scope. Also remove local function declarations, ie follow the documented bottom-up design, see engineering.html, or use C++ static class members. Ensure you remove Examples/test-suite/r/Makefile from your patch too as only Makefile.in gets added to cvs. In expandTypedef, use SwigType_isenum() instead of Strstr. Use FileErrorDisplay instead of Printf when NewFile fails. Suggest using Swig_print_node/Swig_dump_node instead of ShowHash. Suggest looking at copying and using getProxyName instead of getRClassName/getRClassNameCopyStruct methods if it is possible. Please use the same coding style as the rest of the SWIG language modules, eg all the R class methods being inline, brace positions on same line as method declaration etc. Commenting functions which aren't derived from Language would be useful. Function access for the runtime library in rrun.swg needs fixing, ie use SWIGEXPORT, SWIGINTERN, SWIGRUNTIME etc. Welcome to the hard-core swig hackers club! When you check your code in add your name to the README file. Also send a photo for the website, the same goes for anyone else who hasn't got their mugshut on the website yet. William Joseph Wang wrote: > I've attached a 42.5 K tar file that contains the files and the patches that I > want to check in. Most of it includes addition files that need to be checked > in and the changes to the pre-existing code are minimal (mostly just defining > R in the hooks and adding R to an ifdef in the unit tests to handle a class > with no default constructor). > > Let me know if there is anything else you need. I have the logs with patch > cleanly compiling all of the unit tests, but they are quite a bit larger than > the code itself. :-) :-) :-) > > 在 Wednesday 21 June 2006 14:28,William S Fulton 写道: >> Joseph Wang wrote: >>> R-SWIG is at the point where it now compiles all of the C++ unit tests, >>> and has support for all of the functionality that I know of that was in >>> Duncan Temple Lang's original implementation (including copyToR). There >>> are still some work do to which includes removing all of the redundant >>> code and reimplementing function overloading using the SWIG dispatch >>> mechanism instead of S4 methods. >>> >>> But I'd like to have the code under version control before undertaking >>> these changes. The R-SWIG is part of my general research on Shanghai >>> derivatives so I will be committed to supporting the module for an >>> extended time. My sourceforge ID is drjoe. >>> >>> Comments? >> Sounds good. Can you please post a patch of what your initial commit >> will be so we can review it first. >> >> Thanks >> William |
From: John L. <le...@cs...> - 2006-06-23 00:17:54
|
William S Fulton wrote: > Function access for the runtime library in rrun.swg needs fixing, ie use > SWIGEXPORT, SWIGINTERN, SWIGRUNTIME etc. > > Welcome to the hard-core swig hackers club! When you check your code in > add your name to the README file. Also send a photo for the website, the > same goes for anyone else who hasn't got their mugshut on the website yet. I'd like to take a scan at the runtime code side of things, just to see if you are using it correctly. >> I've attached a 42.5 K tar file that contains the files and the patches that I >> want to check in. Most of it includes addition files that need to be checked >> in and the changes to the pre-existing code are minimal (mostly just defining >> R in the hooks and adding R to an ifdef in the unit tests to handle a class >> with no default constructor). I think the swig mailing list killed the message, could you send it to me directly? Thanks John |
From: William S F. <ws...@fu...> - 2006-06-24 00:01:05
|
William S Fulton wrote: > Hi Joseph, > > Looks good, but I have some points which need addressing before you > check any of this in. I'll set up your cvs access when I get a moment > either tonight or tomorrow. > Done. There is also a new category for adding bug called 'r'. Please close any bugs assigned and fixed in this category. Thanks William |