There are still some minor issues with python garbage collection. I
briefly talked about this with Marcus on IRC but I thought it would be
a good idea to write this down and get some feedback.
python scripts return objects which may be destroyed before we use them
Should return a pointer to QUndoCommand. Creating a derived
QUndoCommand derived class in python and returning this works fine as
long as the command isn’t destroyed by python garbage collection. But
even if the script holds a reference, reloading plugins will destroy
the command! However, we still want the command to work when we undo
the last action.
QUndoCommand is a simple class, it doesn’t inherit QObject which means
we can’t connect to a destroyed() signal. Since it is such a simple
class I think the best way to handle this would be to create a
Avogadro.UndoCommand class in the python bindings. This class would
inherit QUndoCommand but boost python allows us to hold the object
using a std::auto_ptr. Next we can release (take ownership) of this
auto_ptr in the PythonExtension::performAction C++ function.
b) PythonTool::settingsWidget & PythonEngine::settingsWidget
This has been solved I think, both PythonTool and PythonEngine wrap
the QWidget extracted (using boost::python::extract) in a C++ created
QWidget. So even when the python script returns a QWidget without
holding a reference to it, the wrapper QWidget is still valid and we
don’t segfault… (note: a Tool or Engine’s settings QWidget never has
to live longer than the plugin itself)
- Who owns or should own Molecule(s)?
While we could add a MoleculeStack or something similar to libavogadro
to own Molecules (currently MainWindow owns molecules), I would just
remove python functions like GLWidget::setMolecule and add a copy
function to the python bindings.
This would look like:
self.molCopy = Avogadro.Molecule(glwidget.molecule) # OK! this
molecule will never be used outside script
glwidget.molecule.copy(self.molCopy) # We don’t actually change the molecule…
Currently we allow:
glwidget.molecule = Avogadro.Molecule(self.molCopy) # this calls
GLWidget::setMolecule, the new Molecule is destroyed right away…
We could make this work using an std::auto_ptr which we release in a
function overloading  GLWidget::setMolecule in the python bindings,
but even then it doesn’t make much sense. For example, we have no way
to tell MainWindow the molecule has changed.
 In case anyone wondered, we can overload any function in the
python bindings like this:
// the first parameter is the object (like self in python)
void GLWidget_setMolecule(GLWidget &glwidget, std::auto_ptr mol)
.add_property(“molecule”, /* get method left out */, &GLWidget_setMolecule)