On Mon, Jan 19, 2009 at 11:19 PM, Petr Novak <shaman@xxxxxxxxxxxxxxxxxx>
wrote:
[snip]
I am working with Pavel Kaderavek on 'relax_csa' branch. Since I am not
experienced coder I use existing code as an inspiration and sort of
'template code' for our own changes/features. Just now I am trying to
implement functions for manipulating with chemical shift tensor data
(cs_tensor.read/copy/delete/display()) to generic_fns. As a template for
these functions I've taken generic_fns/relaxation_data.py. When I got to
delete() and display() I found this in code:
Coding by replication is also how I do most of my coding in relax
nowadays, even for core infrastructure changes. A lot of new
functionality is very similar to the current code, so this makes life
much easier.
As for taking 'generic_fns/relax_data.py' (I'm assuming this is the
file and not 'generic_fns/relaxation_data.py'), is this actually the
best starting point? It gives you code which works at the spin system
level, which is what you need, but the concepts and code in
'generic_fns/diffusion_tensor.py' will also be extremely useful.
Exactly as you said. 'generic_fns/relax_data.py' is template for
spin-level functionality and 'generic_fns/diffusion_tensor.py'
inspiration for tensor-related functionality.
This sounds like the file to copy then.
If 'generic_fns/relax_data.py' is the best, then I will use the
subversion command 'svn cp' to copy 'generic_fns/relax_data.py' to
some other file for you. This preserves the historic links between
the files in the repository so developers now and in the future can
see where you started from - this is essential. So I need to know
what you would like to call the new file. Once I have made the copy,
you can use 'svn up' on your checked out copy of the 'cst' branch to
get the code.
Aforementioned 'duality' in the templates makes me hasitate which file
should be the copied ('generic_fns/relax_data.py' or
'generic_fns/diffusion_tensor.py'). Is it good idea to simply add
'_cst', e.g. 'generic_fns/relax_data_cst.py' ?
The name has of this file has nothing to do with relaxation data. I
would suggest a name related to it's function, something that is also
generalisable for other chemical shift related functions. I would
therefore suggest:
generic_fns/chemical_shift.py
There may be a better suited name though that you could think of.
relax> relax_data.delete(ri_label='R1', frq_label='600')
Traceback (most recent call last):
File "<console>", line 1, in ?
File "/home/shaman/relax/prompt/relax_data.py", line 195, in delete
relax_data.delete(ri_label=ri_label, frq_label=frq_label)
File "/home/shaman/relax/generic_fns/relax_data.py", line 410, in delete
self.run = run
NameError: global name 'run' is not defined
And this is the result. The relaxation data deletion code is
currently non-functional in the 1.3 line. It's not used much and is
not in a system test, hence why it hasn't been converted yet. But if
you convert it for your code, that will be very good for learning the
differences between the two lines.
Yes, I did the conversion for delete() function and after finishing this
email I will do the same for display() function. Do you want me to post
it here or should it be included in our patch ?
For this code, I would suggest one patch per small increment of code
you make - for example fixing the delete() function should be 1 single
patch. Once you have made the patch, you need to write a commit
message for me to use. This is all finely documented in the
development chapter of the relax manual.
I have a question though, how do you know that this function now
works? One part of the new relax design of the 1.3 line, thanks to
Gary Thompson's work with the test suite, is the power and simplicity
of the unit tests and system/functional tests. To check that delete()
works, either a unit test or system test could be written for it.
Then if the function works, the test passes. Otherwise the test
fails. This is extremely useful for coding and debugging, and the
test should ideally be written before coding or fixing the actual
function.
This also applies for the full analysis of the ellipsoidal CS tensor.
Please have a close look at the system test framework, and the scripts
in 'test_suite/system_tests/scripts' to see how this is done. You
should before you start anything have a system test which checks the
full behaviour of the modified analysis - carefully checking the
results to see if it is the same as what your code based on the 1.2
line returns. Do you have a link for that code that you could post
here, just for reference? The test will only pass once absolutely
everything is functional!
Was it you that coded the original
version of the ellipsoidal CS tensor, based on the 1.2 line code?
Yes, it was me. Pavel did implementation of code for equations and I did
the code dealing with reading/handling/writing of data.
Ah, ok. This will be useful to know.
My theory about this is:
a) I missed something important in my python self.education :-) (I
deserted from biochemistry field so it is very probable)
This is relax's design, and nothing to do with python. And my
undergrad background is pure biochemistry as well ;) No chemistry,
physics, maths, statistics, or coding (well apart from first year
classes for all but the coding).
Well, knowing that this "conversion" is doable is inspiring and
motivating :-)
I wouldn't call it a conversion - I still remember all my biology
skills. It can freak some people out when they get a question they
weren't expecting from someone they would least expect to be a
biologist ;)
c) There is always some other option :-P :-)
I'm too slow converting the entire code base :)
As I understood it's now way bigger then original "script" for executing
Model free program and reading its results :-)
There is quite a lot of code in there nowadays!
Can you please comment on this ? Btw do you think it should be deleted
from every pipe or just from 'cdp' ( = pipes.get_pipe() ).
I hope the comments above cover it all. If not, feel free to ask
about anything on this list.
Cheers,
Edward
Thanks for your valuable comments and I'll get back to coding :-)
Before you do, remember that patches should be small and cover only
one concept/idea/fix. If they are over 50 lines of code, I would be
worried. This will slow development for a while, but I need to
carefully check every last line of code. And this is how the
development should be done also when you have commit access. Other
relax developers may also check your code. These small changes should
continually be applied to the repository, one by one. One massive
patch cannot be accepted into an open source project, or any project
for that matter.
Cheers,
Edward
Re: generic_fns.relaxation_data.delete()