Hi Michael,
Please see below for comments. Your coding style is improving nicely!
The comments are of high quality, and the commit messages as well :)
I would just be more careful with spelling, captialisation, and full
stops - proper sentences should be used. I would like to see how you
go with the NOE analysis spinning out, and then I would like call a
vote for your inclusion as a relax developer.
Cheers,
Edward
On 16 February 2010 03:49, Michael Bieri
<NO-REPLY.INVALID-ADDRESS@xxxxxxx> wrote:
Follow-up Comment #35, task #6847 (project relax):
Hi Edward
A couple of new patches:
relax_times:
Relaxation times are correctly read from data storage.
- data.relax_times = self.data.file_list[:i]
+ data.relax_times = self.data.relax_times[:i]
Perfect! This is small, but unrelated to the other changes so needs
to be its own commit.
---------------------------------
execute_rx:
Unresolved Residues and PDB file will be read of data storage and transfered
to relax_fit.py.
@@ -264,6 +264,12 @@
# The number of Monte Carlo simulations to be used for error
analysis at the end of the analysis.
data.mc_num = 500
+ # Unresolved resiudes
+ data.unresolved = self.data.unresolved
+
+ # Structure File
+ data.structure_file = self.data.structure_file
+
# Return the container.
return data
@@ -376,7 +383,7 @@
data = self.assemble_data()
# Execute.
- Relax_fit(seq_args=data.seq_args, file_names=data.file_names,
relax_times=data.relax_times, int_method=data.int_method,
mc_num=data.mc_num)
+ Relax_fit(seq_args=data.seq_args, file_names=data.file_names,
relax_times=data.relax_times, int_method=data.int_method,
mc_num=data.mc_num,
pdb_file = data.structure_file, unresolved = data.unresolved)
def link_data(self, data):
Here there are capitalisation, full stop, and spelling issues in the
comments. You should try to make these as proper sentences, as it
will be read by non-coders. Proper sentences are easier to read, and
tell the story of what the code is doing. This is also useful for
users to instantly spot if the code is doing as you intended.
------------------------------------------------------------
settings_patch:
Option to load a sequence file was added bakc to the settings.
@@ -27,6 +27,7 @@
import wx
# relax GUI module imports.
+from filedialog import openfile
from paths import IMAGE_PATH
@@ -51,7 +52,7 @@
def load_sequence(self):
- seqfile = openfile('Choose Sequence File', sys.path[-1],
'save.relaxGUI', 'relaxGUI files (*.relaxGUI)|*.relaxGUI|all files
(*.*)|*.*')
+ seqfile = openfile('Choose Sequence File', sys.path[-1], '', 'all files
(*.*)|*.*')
return seqfile
This is good. Just one spelling mistake in the commit message.
------------------------------------------------------------
settings_read:
Stored settings are now synchronized with settings dialog for relax settings
and peak file settings.
@@ -1008,11 +1009,10 @@
def param_file_setting(self, event): # set up parameter files
- global file_setting # import global variable
- tmp_setting = import_file_settings(file_setting)
+ tmp_setting = import_file_settings(ds.relax_gui.file_setting)
if not tmp_setting == None:
if question('Do you realy want to change import file
settings?'):
- file_setting = tmp_setting
+ ds.relax_gui.file_setting = tmp_setting
event.Skip()
@@ -1140,10 +1140,9 @@
def reset_setting(self, event): #reset all settings
global global_setting #import global variable
- global file_setting # import global variable
if question('Do you realy want to change relax settings?'):
- global_setting = ['1.02 * 1e-10', '-172 * 1e-6', 'N', 'H',
'11',
'newton', '500']
- file_setting = ['0', '1', '2', '3', '4', '5', '6']
+ ds.relax_gui.global_setting = ['1.02 * 1e-10', '-172 * 1e-6',
'N', 'H', '11', 'newton', '500']
+ ds.relax_gui.file_setting = ['0', '1', '2', '3', '4', '5', '6']
def sat_noe1(self, event):
@@ -1228,11 +1227,10 @@
def settings(self, event): # set up for relax variables
- global global_setting #import global variable
- tmp_global = relax_global_settings(global_setting)
+ tmp_global = relax_global_settings(ds.relax_gui.global_setting)
if not tmp_global == None:
if question('Do you realy want to change relax settings?'):
- global_setting = tmp_global
+ ds.relax_gui.global_setting = tmp_global
event.Skip()
This won't apply cleanly? I'm not sure why? I have manually made the
change. In the future, can you specify the exact repository revision
that the patch applies to? This will let me apply patches cleanly.
Cheers.
----------------------------------------------------------
relax_fit:
This one is complex. The code should actually go into the 1.3 line as
that is where this common code is actually shared. Once the 1.3 line
is changed, the svnmerge program for branch management can be used to
bring it back in:
$ svnmerge merge
$ svn ci -F svnmerge-commit-message.txt
This should also be a number of patches, for example the data and time
addition to the pipe name should be one patch.
Several minor fixes for the relax_fit.py script allow running the analysis
up
to minimisation.
Included:
Import of PDB file and unresolved residues
The type of these should be 'str or file object'. This allows you in
the GUI to build a file object with the unresolved peak info and pass
that in. The unresolved arg should default to None (and no spaces
should be between it, '=', and the default value). Then the check
should be:
if self.unresolved:
self.interpreter.deselect.read(file=self.unresolved)
and it should not use the text 'unresolved'. There should be no print
outs in here, that is up to the user functions (clean API separation).
CRITICAL: For the PDB file, we have to some how in the GUI allow the
user to either load the sequence through a sequence file (with them
specifying the column numbers, column separator, etc.) or through the
PDB file. This will require some GUI redesigning. The auto_analyses
code can do the following: If a sequence file is given, read that
with that file's specific settings (column numbers, column separator,
etc.); Otherwise read it from the PDB file. The GUI should warn if
not enough info is given for this.
Oh, the line:
generic_fns.structure.main.load_spins(spin_id='@N')
should be
self.interpreter.structure.load_spins(spin_id='@N')
This passes the code flow through the user function so that you get a
print out. This will also allow the user functions to place their
name upon execution into the status object (that the relax controller
can tap into).
Also print outs should not be here. Temporary debugging print outs
can be used but should not be in a commit. If the auto analysis is
doing something unusual, then print outs are acceptable (i.e. the
convergence checks in the dAuvergne_protocol code).
Pipe name was supplemented with date and time, so users can start multiple
calculations in the same session (avoiding 'pipe already exists')
This is a brilliant idea! I was wondering how we could do this, and
using the time string is better than my idea of using a random
character string! It should be 1 patch for the 1.3 line though.
Added proton and hetero nucleus information to spectrum.read_intensities()
This is not quite right. The 'N' and 'H' default values should be
done differently. These should be args to the Relax_fit class, and
then self.proton, and self.heteronuc should be passed in to the user
function. They should default to None - I don't want relax to
preference proteins over other molecules. The issue here is that if
you are working with another system and leave the defaults on, you
will be confused as to why relax will not run! It has happened many
times before. I will try to get some CH and NH RNA data for you to
test the GUI on.
Replicated spectra are set correctly.
This is a good bug fix and should be one patch.
Only deselect spins if specified (needs more fixes!)
See above. This patch will not be applied in its current form.
Re: [task #6847] The Bieri graphical user interface.