From: Soeren S. <so...@de...> - 2014-02-25 07:36:29
|
On 24.02.14 20:24, William S Fulton wrote: > On 24/02/14 07:14, Soeren Sonnenburg wrote: >> Hi everyone, >> >> checking a swig wrapper I realized that all >> >> static PyHeapTypeObject SwigPyBuiltin_* { >> ... >> "MajorityVote", /* tp_name */ >> ... >> }; >> >> >> do not contain the package etc as int he recommendation here: >> >> http://docs.python.org/2/c-api/typeobj.html >> >> >> char* PyTypeObject.tp_name >> Pointer to a NUL-terminated string containing the name of the type. For >> types that are accessible as module globals, the string should be the >> full module name, followed by a dot, followed by the type name; for >> built-in types, it should be just the type name. If the module is a >> submodule of a package, the full package name is part of the full module >> name. For example, a type named T defined in module M in subpackage Q in >> package P should have the tp_name initializer "P.Q.M.T". >> >> Would it be OK to fix this? >> > > Seems to make sense. Does it actually cause failures in some way atm? No it is totally harmless at least so far and we have been using shogun with the python2 swig wrapped extensions for years with that w/o trouble and now python3 too. It is just that when you do type(obj) you get the type w/o the module name which is not what one would expect. I only noticed when trying to get pickle to work with swig wrapped shogun objects. And since you have the package name anyway sitting there it is trivial to fix. > I would advise checking master HEAD rather than 2.0.12 before doing so as > Paweł Tomulik <pto...@me...> revamped a related area (import > statements) in a big way. See https://github.com/swig/swig/pull/7. See > 2013-12-24 entry in CHANGES.current file for a summary pointing to some > docs. I'll be somewhat surprised if we don't have a testcase for this. > Paweł, you put in a lot of tests around this area, what do you think? So if you are OK either I would send a PR or you do it yourself... Best, Soeren |