| msg289245 - (view) |
Author: Tristan Croll (Tristan Croll) |
Date: 2017-03-08 16:47 |
| Possibly related to http://bugs.python.org/issue29327 - yields the same error message: Objects/tupleobject.c:81: bad argument to internal function I have a large SWIG project which was previously working well in Python 3.5. After migrating to Python 3.6.0, I find I can still create any wrapped object from Python via its constructor(s), but any internal function that returns certain objects fails with the above message. I have so far been unable to find any distinction between classes that do and don't return successfully. Take the below (and attached) headers, for example. Functions that return Spacegroup objects work, as do those that return Metric_tensor objects from the attached cell.h. On the other hand, functions returning Cell or Cell_descr objects fail with the above message. Yet in all cases I can successfully call the objects' constructors. Not ashamed to say I'm a bit lost here. #ifndef CLIPPER_SPACEGROUP #define CLIPPER_SPACEGROUP #include "symop.h" #include "spacegroup_data.h" namespace clipper { // forward definitions class HKL; class HKL_class; class Coord_frac; //! spacegroup description /*! The spacegroup description is a compact description of a spacegroup. It may be initialised from Hall or H-M symbols, a string of symops or a number. Internally a hash code is used to refer to the spacegroup, so this object is only 32 bits in size. For more details of spacegroup symbols, see Sydney R. Hall & Ralf W. Grosse-Kunstleve 'Concise Space-Group Symbols', http://www.kristall.ethz.ch/LFK/software/sginfo/hall_symbols.html */ class Spgr_descr { public: enum TYPE { Hall, HM, XHM, Symops, Number, Unknown }; //! null constructor Spgr_descr(); //! constructor: from symbol or operators. explicit Spgr_descr( const String& symb, TYPE type = Unknown ); //! constructor: from number. explicit Spgr_descr( const int& num ); //! return the spacegroup number int spacegroup_number() const; //! return the Hall symbol String symbol_hall() const; //! return the H-M symbol String symbol_hm() const; //! return the extended H-M symbol String symbol_xhm() const; //! return the extension H-M symbol String symbol_hm_ext() const; //! set preferred default spacegroup choice static void set_preferred( const char& c ); //! Vector of symop codes and associated methods class Symop_codes : public std::vector<Symop_code> { public: //! initialise from Hall symbol void init_hall( const String& symb ); //! initialise from symops void init_symops( const String& symb ); //! expand (incomplete) list of symops Symop_codes expand() const; //! return primitive non-inversion ops (by computation) Symop_codes primitive_noninversion_ops() const; //! return inversion ops (by computation) Symop_codes inversion_ops() const; //! return primitive incl inversion ops (by computation) Symop_codes primitive_ops() const; //! return lattice centering ops (by computation) Symop_codes centering_ops() const; //! return Laue ops Symop_codes laue_ops() const; //! return point group ops Symop_codes pgrp_ops() const; //! return Patterson ops Symop_codes patterson_ops() const; //! return minimal list of generator ops Symop_codes generator_ops() const; //! return product of this (expanded) list by another (expanded) list Symop_codes product( const Symop_codes& ops2 ) const; //! return hash code of symop list unsigned int hash() const; }; //! constructor: from symop list. explicit Spgr_descr( const Symop_codes& ops ); //! return the generators for the spacegroup const Symop_codes& generator_ops() const { return generators_; } //! return the hash code for the spacegroup \internal const unsigned int& hash() const { return hash_; } protected: unsigned int hash_; //!< hash code of spacegroup Symop_codes generators_; //!< codes for symop generators static char pref_12, pref_hr; //!< preferred origin and hex/romb symbols }; // ObjectCache data type class Spgr_cacheobj { public: typedef Spgr_descr Key; Spgr_cacheobj( const Key& spgr_cachekey ); //!< construct entry bool matches( const Key& spgr_cachekey ) const; //!< compare entry String format() const; //!< string description // data Key spgr_cachekey_; //!< spacegroup cachekey int nsym, nsymn, nsymi, nsymc, nsymp; //!< number of syms: total, primitive int lgrp; //!< Laue group number std::vector symops; //!< symmetry operators std::vector isymops; //!< symmetry operators Vec3<> asu_min_, asu_max_; //!< real space ASU static Mutex mutex; //!< thread safety }; //! Spacegroup object /*! The spacegroup object is a full description of a spacegroup, including all the most regularly used information in an efficient form. It may be initialised from a clipper::Spgr_descr. This object. For more details of spacegroup symbols, see Sydney R. Hall & Ralf W. Grosse-Kunstleve 'Concise Space-Group Symbols', http://www.kristall.ethz.ch/LFK/software/sginfo/hall_symbols.html */ class Spacegroup : public Spgr_descr { public: //! enumeration for fast construction of Null or P1 spacegroup enum TYPE { Null, P1 }; //! enumeration for cell axes enum AXIS { A=0, B=1, C=2 }; //! null constructor Spacegroup() {}; //! constructor: fast constructor for Null or P1 spacegroup explicit Spacegroup( TYPE type ); //! constructor: from spacegroup description explicit Spacegroup( const Spgr_descr& spgr_descr ); //! initialiser: from spacegroup description void init( const Spgr_descr& spgr_descr ); //! test if object has been initialised bool is_null() const; // methods //! get spacegroup description inline const Spgr_descr& descr() const { return (*this); } //! get number of symops inline const int& num_symops() const { return nsym; } //! get number of primitive symops (identical to num_primitive_symops()) inline const int& num_primops() const { return num_primitive_symops(); } //! get number of primitive symops (inc identity and inversion) inline const int& num_primitive_symops() const { return nsymp; } //! get number of centering symops (inc identity) inline const int& num_centering_symops() const { return nsymc; } //! get number of inversion symops (inc identity) inline const int& num_inversion_symops() const { return nsymi; } //! get number of primitive non-inversion symops (inc identity) inline const int& num_primitive_noninversion_symops() const { return nsymn;} //! get n'th symop inline const Symop& symop( const int& sym_no ) const { return symops[sym_no]; } //! get n'th primitive symop (identical to symop(sym_no)) inline const Symop& primitive_symop( const int& sym_no ) const { return symops[sym_no]; } //! get n'th inversion symop (0...1 max) inline const Symop& inversion_symop( const int& sym_no ) const { return symops[nsymn*sym_no]; } //! get n'th centering symop (0...3 max) inline const Symop& centering_symop( const int& sym_no ) const { return symops[nsymp*sym_no]; } //! get the order of rotational symmetry about a given axis int order_of_symmetry_about_axis( const AXIS axis ) const; //! get 'class' of reflection: multiplicity, allowed phase, absence HKL_class hkl_class( const HKL& hkl ) const; //! test if hkl is in default reciprocal ASU bool recip_asu( const HKL& hkl ) const; //! get symop number corresponding to the product of two symops int product_op( const int& s1, int& s2 ) const; //! get symop number corresponding to the inverse of a symop int inverse_op( const int& s ) const; //! get map ASU, upper bound Coord_frac asu_max() const; //! get map ASU, lower bound Coord_frac asu_min() const; //! test if change of hand preserves spacegroup bool invariant_under_change_of_hand() const; // inherited functions listed for documentation purposes //-- int spacegroup_number() const; //-- String symbol_hall() const; //-- String symbol_hm() const; //! return the Laue group symbol String symbol_laue() const; //! Return P1 spacegroup static Spacegroup p1() { return Spacegroup( P1 ); } //! Return null spacegroup static Spacegroup null() { return Spacegroup( Null ); } void debug() const; private: ObjectCache<Spgr_cacheobj>::Reference cacheref; //!< object cache reference const Symop* symops; //!< fast access ptr const Isymop* isymops; //!< fast access ptr data::ASUfn asufn; //!< fast access ptr int nsym, nsymn, nsymi, nsymc, nsymp; //!< fast access copies }; } // namespace clipper #endif |
|
|
| msg289280 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-03-09 09:15 |
| Which function raises this exception? Add the traceback place. It may be a bug in your code. |
|
|
| msg289281 - (view) |
Author: Tristan Croll (Tristan Croll) |
Date: 2017-03-09 10:10 |
| OK, a further clue. First, a little more detail on how my project is arranged (to re-iterate, this works without complaint in Python 3.5): Rather than use my SWIG output directly, I've created a further wrapper layer in Python to add functions/syntactic sugar that would be difficult to achieve in SWIG, add a bit of extra documentation, and bury some classes that may be occasionally useful but don't really need to be in the primary API. The SWIG output Python library I name clipper_core, and I use the following class decorators for classes in the front-end API to reduce the amount of code (Python and SWIG) needed: def mappedclass(old_cls): ''' Ensures that objects returned from functions in the clipper_core library are instantiated in Python as the derived class with the extra functionality. ''' def decorator(cls): def __newnew__(thiscls, *args, **kwargs): if thiscls == old_cls: return object.__new__(cls) return object.__new__(thiscls) old_cls.__new__ = __newnew__ return cls return decorator def getters_to_properties(*funcs): ''' Class decorator. Add the names of any getter functions with void arguments (e.g. Coord_grid.u()) to convert them to properties. If you want the property name to be different from the function name, add the desired name and the function name as a tuple (e.g. ('uvw', '_get_uvw')) ''' def property_factory(func): def getter(self): return getattr(super(self.__class__, self), func)() prop = property(getter) return prop def decorator(cls): for func in funcs: if type(func) == tuple: setattr(cls, func[0], property_factory(func[1])) else: setattr(cls, func, property_factory(func)) return cls return decorator def format_to_string(cls): ''' Class decorator to redirect the Clipper format() function to __str__, to provide pretty printing of the object. ''' def format(self): return super(self.__class__,self).format() def __str__(self): return self.format setattr(cls, 'format', property(format)) setattr(cls, '__str__', __str__) return cls Experimenting this morning with the following two classes: @format_to_string @mappedclass(clipper_core.Cell_descr) class Cell_descr(clipper_core.Cell_descr): def __init__(self, abc, angles): ''' __init__(self, abc, angles) -> Cell_descr Args: abc ([float*3]): cell dimensions in Angstroms angles ([float*3]): alpha, beta and gamma angles in degrees ''' clipper_core.Cell_descr.__init__(self, *abc, *angles) @format_to_string @getters_to_properties('cell_descr', 'matrix_frac', 'matrix_orth', 'metric_real', 'metric_reci', 'volume') @mappedclass(clipper_core.Cell) class Cell(clipper_core.Cell): ''' Define a crystallographic unit cell using the lengths of the three sides a, b, c (in Angstroms) and the three angles alpha, beta, gamma (in degrees). ''' def __init__(self, abc, angles): ''' __init__(self, abc, angles) -> Cell Args: abc ([float*3]): cell dimensions in Angstroms angles ([float*3]): alpha, beta and gamma angles in degrees ''' cell_descr = Cell_descr(abc, angles) #cell_descr = clipper_core.Cell_descr(*abc, *angles) clipper_core.Cell.__init__(self, cell_descr) def __eq__(self, other): return self.equals(other) Then: import clipper cell = clipper.cell([100,100,100],[90,90,90]) cell.cell_descr SystemError Traceback (most recent call last) in () ----> 1 c.cell_descr /home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/clipper/clipper_decorators.py in getter(self) 87 def property_factory(func): 88 def getter(self): ---> 89 return getattr(super(self.__class__, self), func)() 90 prop = property(getter) 91 return prop /home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/clipper/lib/clipper_python_core/clipper.py in cell_descr(self) 8249 8250 """ -> 8251 return _clipper.Cell_cell_descr(self) 8252 8253 __swig_destroy__ = _clipper.delete_Cell SystemError: Objects/tupleobject.c:81: bad argument to internal function ... but if I comment out the derived Cell_descr class and switch to the alternate cell_descr constructor in Cell.__init__(), then it works as expected. I have tried commenting out the other class decorators, with no effect - so it would seem it's what's happening in the @mappedclass decorator that is causing my troubles. |
|
|
| msg289314 - (view) |
Author: Tristan Croll (Tristan Croll) |
Date: 2017-03-09 17:30 |
| OK, this seems to narrow down the problem. The following was legal in Python 3.5.1, but in 3.5.3 and 3.6.1rc1 returns: 'TypeError: must be type, not classobj' class Foo_Base: pass class Bar_Base: def get_foo(self): f = Foo_Base() return f class Foo(Foo_Base): pass class Bar(Bar_Base): def get_foo2(self): return super(Bar, self).get_foo() bar = Bar() b = bar.get_foo2() Is this a deliberate and permanent change? If so, it looks like I have a lot of work on my hands. |
|
|
| msg289317 - (view) |
Author: Tristan Croll (Tristan Croll) |
Date: 2017-03-09 17:39 |
| Nope - belay that. Checking through the SWIG-generated Python code, all the classes correctly inherit from object, which negates that issue. |
|
|
| msg289318 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-03-09 17:50 |
| The code in doesn't emit any warning. I tested 3.5, 3.6 and master development branches and system Python 3.5.2. |
|
|
| msg289319 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-03-09 17:51 |
| "Nope - belay that. Checking through the SWIG-generated Python code, all the classes correctly inherit from object, which negates that issue." I don't understand your comment, on Python 3, any class inherit from object be default. There is no more old and new classes. haypo@selma$ cat y.py class A: pass class B(object): pass print(A.__bases__) print(B.__bases__) haypo@selma$ python3 y.py (<class 'object'>,) (<class 'object'>,) |
|
|
| msg289320 - (view) |
Author: Tristan Croll (Tristan Croll) |
Date: 2017-03-09 17:52 |
| Sorry - ignore that. Brain-fart at the end of a (very) long day. |
|
|
| msg289322 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2017-03-09 18:39 |
| Since it sounds like everything on the Python side is fine I'm closing this. |
|
|
| msg289323 - (view) |
Author: Tristan Croll (Tristan Croll) |
Date: 2017-03-09 18:51 |
| I don't agree that it should be closed yet. I still have the issue that an approach that was perfectly legal in Python 3.5 now no longer works in Python 3.6, and I don't know why. The description in stands, and is a real problem. Nothing has changed in my code - I compile against Python 3.5 and it works, against 3.6 it doesn't. |
|
|
| msg289324 - (view) |
Author: Tristan Croll (Tristan Croll) |
Date: 2017-03-09 19:11 |
| I've cross-posted the following to the SWIG bug tracker. Hopefully someone can find an answer, because I'm getting nowhere. If I have two classes Foo and Bar (where Bar has a function get_foo() that returns a Foo object) defined in the SWIG-generated library foobar, and wrap them as follows: ``` def mappedclass(old_cls): ''' Ensures that objects returned from functions in the clipper_core library are instantiated in Python as the derived class with the extra functionality. ''' def decorator(cls): def __newnew__(thiscls, *args, **kwargs): if thiscls == old_cls: return object.__new__(cls) return object.__new__(thiscls) old_cls.__new__ = __newnew__ return cls return decorator @mappedclass(foobar.Foo) class Foo(foobar.Foo): pass @mappedclass(foobar.Bar) class Bar(foobar.Bar): def get_foo(self): return super(Bar, self).get_foo() ``` then in Python 3.5: ``` f = Foo() b = Bar() b.get_foo() ``` all work. In Python 3.6: ``` f = Foo() b = Bar() ``` ... both work, but `b.get_foo()` yields the error as per my OP. Real-world examples: Constructor (works without trouble in both versions) ``` SWIGINTERN PyObject *_wrap_new_Cell_descr__SWIG_0(PyObject *SWIGUNUSEDPARM(self), PyObject *args) { PyObject *resultobj = 0; clipper::Cell_descr *result = 0 ; if(!PyArg_UnpackTuple(args,(char *)"new_Cell_descr",0,0)) SWIG_fail; { try { result = (clipper::Cell_descr *)new clipper::Cell_descr(); } catch (clipper::Message_fatal m) { SWIG_exception(SWIG_RuntimeError, m.text().c_str() ); SWIG_fail; } catch (std::out_of_range e) { const char *errString; if ( !strcmp(e.what(), "" ) ) { errString = "Index out of range!"; } else { errString = e.what(); } SWIG_exception(SWIG_IndexError, errString ); SWIG_fail; } catch (std::length_error e) { SWIG_exception(SWIG_ValueError, e.what() ); SWIG_fail; } catch (std::invalid_argument e) { SWIG_exception(SWIG_ValueError, e.what() ); SWIG_fail; } catch (std::exception e) { SWIG_exception(SWIG_UnknownError, e.what() ); SWIG_fail; } catch (...) { SWIG_exception(SWIG_UnknownError, "Unknown error" ); SWIG_fail; } } resultobj = SWIG_NewPointerObj(SWIG_as_voidptr(result), SWIGTYPE_p_clipper__Cell_descr, SWIG_POINTER_NEW | 0 ); return resultobj; fail: return NULL; } ``` Getter (fails under the above conditions): ``` SWIGINTERN PyObject *_wrap_Cell_cell_descr(PyObject *SWIGUNUSEDPARM(self), PyObject *args) { PyObject *resultobj = 0; clipper::Cell *arg1 = (clipper::Cell *) 0 ; void *argp1 = 0 ; int res1 = 0 ; PyObject * obj0 = 0 ; clipper::Cell_descr result; if(!PyArg_UnpackTuple(args,(char *)"Cell_cell_descr",1,1,&obj0)) SWIG_fail; res1 = SWIG_ConvertPtr(obj0, &argp1,SWIGTYPE_p_clipper__Cell, 0 |
0 ); if (!SWIG_IsOK(res1)) { SWIG_exception_fail(SWIG_ArgError(res1), "in method '" "Cell_cell_descr" "', argument " "1"" of type '" "clipper::Cell *""'"); } arg1 = reinterpret_cast< clipper::Cell * >(argp1); { try { result = clipper_Cell_cell_descr(arg1); } catch (clipper::Message_fatal m) { SWIG_exception(SWIG_RuntimeError, m.text().c_str() ); SWIG_fail; } catch (std::out_of_range e) { const char *errString; if ( !strcmp(e.what(), "" ) ) { errString = "Index out of range!"; } else { errString = e.what(); } SWIG_exception(SWIG_IndexError, errString ); SWIG_fail; } catch (std::length_error e) { SWIG_exception(SWIG_ValueError, e.what() ); SWIG_fail; } catch (std::invalid_argument e) { SWIG_exception(SWIG_ValueError, e.what() ); SWIG_fail; } catch (std::exception e) { SWIG_exception(SWIG_UnknownError, e.what() ); SWIG_fail; } catch (...) { SWIG_exception(SWIG_UnknownError, "Unknown error" ); SWIG_fail; } } resultobj = SWIG_NewPointerObj((new clipper::Cell_descr(static_cast< const clipper::Cell_descr& >(result))), SWIGTYPE_p_clipper__Cell_descr, SWIG_POINTER_OWN |
0 ); return resultobj; fail: return NULL; } ``` |
| msg289399 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2017-03-10 18:19 |
| Please keep this issue closed until you hear back from the SWIG team. Just because your code worked under Python 3.5 doesn't mean SWIG didn't accidentally emit something that breaks under Python 3.6 because we started being more stringent about something. Basically unless we have C code (and not SWIG or C++ code) to reproduce this then we have to go on the assumption it's a problem on SWIG's side. |
|
|