Hi Michael, Here are some comments about the relax_gui.py file. Most is formatting for now. I'll break it into numbered sections for reference. Note that each change should probably be a separate patch file with separate commit message describing the change in detail. Commits should be relatively small for this type of development. These comments are general and apply to the rest of the relax code base, and to all code all relax developers write. Sorry for the length, but this should help with all your code. I haven't made major comments about the code structure yet, this just covers formatting and basic structure. Cheers, Edward 1) A module docstring would be useful at the very top (after the copyright message). Maybe a bit like: # relax module docstring. """The Bieri Graphical User Interface for relax.""" 2) Imports. 2a) The 'res' imports should be shifted to the relax module section. 2b) The python module imports should be alphabetical. 2c) The "from xxx import *" statement should never be used. * needs to be replaced by only those module objects directly used in relax_gui.py. 3) Global variables should be all capitalised. These are more like constants. There should be spacing after all commas. See 5) and 11). 4) Field strength defaults in the variables. This is not for changing right now, but I would like to help you design the interface so that you can have say 5 field strengths. And maybe have some data missing, i.e. you collect on 500, 600, 700, 800, and 900, but the 500 and 600 NOE values are missing. This will eliminate the relaxation related variables. 5) Preparative formatting for Python3. See the file 'docs/2to3_checklist'. On each file, execute: $ 2to3 -w \ -f buffer \ -f idioms \ -f set_literal \ -f ws_comma \ -x except \ -x import \ -x imports \ -x long \ -x numliterals \ -x xrange \ my_file.py This should be one patch, with all files updated. This will pick up the missing space after commas. 6) Function docstrings are missing. See other parts of relax for how this is done, e.g. in specifc_fns/model_free/main.py. Epydoc formatting is required. The comment above the function should be the basis of the docstring, i.e. move it into a docstring. See 9) for example. 7) Functions should be alphabetically arranged. 8) Spacing is incorrect within the functions, classes, loops, etc. There should only be 4 space characters for each indent level. See 9) for example. 9) Comment frequency is far too low. In relax there is a policy for commenting everything. As an example: ----- #Results def see_results(openfile): if '.agr' in openfile: os.system('xmgrace ' + openfile + ' &') if '.txt' in openfile: os.system('gedit ' + openfile + ' &') if '.pml' in openfile: os.system('pymol ' + openfile + ' &') ----- def see_results(file_name): """Display the results through an external program. The following programs are used to view the results: - Grace, a plotting program (http://plasma-gate.weizmann.ac.il/Grace/). - Gedit, a text editor (http://projects.gnome.org/gedit/). - PyMOL, a molecular viewer (http://pymol.sourceforge.net/). @param file_name: The name of the results file to open by the external program. @type file_name: str """ # View the results as a Grace plot. if '.agr' in file_name: os.system('xmgrace %s &' % file_name) # Open the results in the Gedit text editor. if '.txt' in file_name: os.system('gedit %s &' % file_name) # Display the results on the molecule structure. if '.pml' in file_name: os.system('pymol %s &' % file_name) ----- The idea here is to describe what the code does in basic English terms. The reason for this is so that non-coders can help contribute to relax. It helps them understand and follow the flow and execution of the code. The more explanations in comments given, the better! 10) create_save_file() should be better formatted. The lists and strings at the start are a debugging nightmare. Try something like: ----- # NOE savenoe1 = [str(self.nmrfreq_value_noe1.GetValue()), str(self.noe_sat_1.GetValue()), str(self.noe_sat_err_1.GetValue()), str(self.noe_ref_1.GetValue()), str(self.noe_ref_err_1.GetValue()), str(self.structure_noe1.GetValue()), replace(str(self.unres_noe1.GetValue()), ',',';'), str(self.res_noe1.GetValue())] ----- # NOE savenoe1 = [str(self.nmrfreq_value_noe1.GetValue()), str(self.noe_sat_1.GetValue()), str(self.noe_sat_err_1.GetValue()), str(self.noe_ref_1.GetValue()), str(self.noe_ref_err_1.GetValue()), str(self.structure_noe1.GetValue()), replace(str(self.unres_noe1.GetValue()), ',',';'), str(self.res_noe1.GetValue()) ] ---- And for the strings: ----- #T1 t1_list_1 = str(self.t1_list_1.GetLabel()) + ', ' + str(self.t1_list_2.GetLabel()) + ', ' + str(self.t1_list_3.GetLabel()) + ', ' + str(self.t1_list_4.GetLabel()) + ', ' + str(self.t1_list_5.GetLabel()) + ', ' + str(self.t1_list_6.GetLabel()) + ', ' + str(self.t1_list_7.GetLabel()) + ', ' + str(self.t1_list_8.GetLabel()) + ', ' + str(self.t1_list_9.GetLabel()) + ', ' + str(self.t1_list_10.GetLabel()) + ', ' + str(self.t1_list_11.GetLabel()) + ', ' + str(self.t1_list_12.GetLabel()) + ', ' + str(self.t1_list_1_copy_11.GetLabel()) + ', ' + str(self.t1_list_14.GetLabel()) ----- #T1 t1_list_1 = ("%s, "*13 + "%s") % str(self.t1_list_1.GetLabel()) str(self.t1_list_2.GetLabel()) str(self.t1_list_3.GetLabel()) str(self.t1_list_4.GetLabel()) str(self.t1_list_5.GetLabel()) str(self.t1_list_6.GetLabel()) str(self.t1_list_7.GetLabel()) str(self.t1_list_8.GetLabel()) str(self.t1_list_9.GetLabel()) str(self.t1_list_10.GetLabel()) str(self.t1_list_11.GetLabel()) str(self.t1_list_12.GetLabel()) str(self.t1_list_1_copy_11.GetLabel()) str(self.t1_list_14.GetLabel()) ----- 11) Formatting is a mess. This is related to 8). For example the white space at the end of lines should be removed. All problems are identified by running in the base directory the command: $ ./scripts/code_validator gui_bieri/relax_gui.py The output of this script will be very useful for cleaning the code up to meet relax's high standards. 12) Statements testing booleans (True and False) can been simplified. E.g. "if self.m0.GetValue():" instead of "if self.m0.GetValue() == True:". 13) The methods of the class "main" are a bit heavy. __init__(), __set_properties(), and __do_layout() have huge amounts of code without comments. I would consider breaking these into small methods and vigorously commenting as to what GUI elements you are building, and what everything is doing. This is very hard for me to follow, so a non-coder would be totally lost in there. 14) The class "main" should be changed to "Main". This will be identified by the code validator script in 11). 15) The class "Model" is defined within the 'start_model_free()' function. This should never be done. Functions and classes should be in the scope of the module, not the functions, classes or methods of a module (rare exceptions exist, but not for something like this). This class could probably go into its own module. 15) This module is 4126 lines long. I would recommend splitting it up into smaller components related to the various functions. I.e. the class "main" could go in it's own module (or 2).