On 11/21/06, Gary S. Thompson <garyt@xxxxxxxxxxxxxxx> wrote:
Edward d'Auvergne wrote:
> That looks awesome, thank you for porting the code into the
> conventions used in relax. The changes will significantly help with
> the API docs, internal help system, etc. and will help standardise
> these docs. I have to apologise for the tediousness of these
> standards - I realise this is not what you would normally expect from
> source code. The reason for these conventions is that I'm trying to
> design the relax source code to be highly readable to allow NMR
> spectroscopists, most of which are either not programmers or have just
> a little programming experience, to be able to easily understand the
> code and extend it's capabilities. The point is to lower the barrier
> of entry into relax so that non-programmers can read the code and
> mimic it to add features that they find useful. Because this is a
> true open source project, many people will eventually read the code.
> The number of people looking at the code is amplified by the fact the
> users are NMR spectroscopists who are generally not afraid to dive
> into code to find problems.
>
Thats fine and in fact a lot of opensource projects have quite strict
conventions on layout. Though most don't expect as much commenting ;-)
But the reasons for that are fair enough here (though there can be
problems with comments and code getting out of sync, which is why most
programmer prefer clear code to comments ;-))
It's not too hard to keep the comments in sync, especially when the
code tells you how while the comments tell you why :)
> Once integrated into
> relax, I think the 'get_startup_path()' can be dropped as
> 'sys.path[-1]' will give the same result.
I would like to avaoid that. The framework is designed so that you will
be able to run parts of the relax unit tests without invoking relax...
(i.e. it will have a 'main' function) and the only available convention
to identify the startup directory is to look at sys.path[0]. The other
thing to say here is that I guess that sys.path[0] ought to stay as the
startup directory as this is the python convention (is there a pressing
need to remove the relax path from the pythonpath?) Also
get_startup_path() also allows for the case when sys.path[0] == ''
which implies the current working directory. Finally if relax does need
to put something before the startup path I guess the other alternative
would be to salt the original path away somewhere else and pass it to
test_runner and thence to test_finder. I could then remove the
dependancy between test_finder and 'get_startup_path()
I've already added the ability to run the system/functional tests
followed by the unit tests through the '--test-suite' command line
option (in the 'test_suite' branch). It shouldn't hurt to allow both
methods of running the tests. For the system/functional tests having
a running instance of relax, either controlling or controlled by the
test suite, is essential anyway.
As for 'sys.path[-1]' being the relax base directory and 'sys.path[0]'
being '.', this is currently only being used by the system/functional
tests to access the path in which the test data is located. I just
looked back at the history and this is some of the most ancient code
in relax - it was introduced in relax version 0.1 at r69 back on Dec
21st, 2001!!! I'm pretty sure this is not necessary any more!
If this code is removed, then all instances of 'sys.path[-1]' could be
replaced by 'sys.path[0]'. To allow the unit tests to be run as part
of the relax test suite or separately, the function
'get_startup_path()' can't hurt. I'll take back everything I said
previously.
> Although I don't have the
> time to do this yet (I will find the time in the future), many of
> these functions could be utilised by the system/function tests. In
> the future they could possibly be shifted into a base class used by
> both the unit and system/function tests.
This was my intention (chris and I talked quite alot about the best way
to do this) and one of the next things on my list was to migrate one of
the system test to use the 'unit test' framework. The reason the
unit_test_runner is where it is is so that I have some unit tests to
hand for testing I am intending to move it further up the hierachy.
I have started to look into using the unittest framework for the
system/functional tests. I do envisage a different layout than the
unit tests though. For instance I'm wondering if I'll need to code a
replacement or subclass the TextTestRunner class. Do you know if
TextTestRunner catches both stdout and stderr? The relax printout
messages need to be suppressed when a test passes. If a failure does
occur, the error object contains the traceback but I would like to
have the normal program printout presented followed by the traceback
for debugging purposes.
I would like a clear distinction between the different classes of
tests so that relax can generate a report in the end. I.e. all the
unit tests are clustered into a single TestSuite (which can be
composed of multiple TestSuite objects for grouping) and all the
system/functional tests clustered into a single separate TestSuite
object. Although the system/functional tests could possibly use the
unittest framework, the unit tests and system tests don't have to be
within the same framework.
I envisage the system tests to work quite differently in that the
instance 'self.relax' is passed into each test class (this is
essential). Most of the automatic stuff in the unit tests will be
unnecessary. For example functions such as
'get_module_relative_path()', etc. will be unnecessary. Actually on
the subject of these functions, the ones not defined as class methods,
they could all be shifted into the classes and accessed by typing
'self.get_module_relative_path()'. Apart from the print statements at
the end of 'unit_test_runner.py', these functions are only accessed by
the other class methods. These may as well be converted to an object
oriented approach.
> I'll look into this once I
> attempt to migrate the system/function tests to use parts or all of
> the unittest framework. At the moment though, we could simply have
> relax run the unit tests followed by the system/function tests (or the
> other way around).
The intention was to have a functional test that ran the unit_tester ;-)
It could be implemented that way. It may be better to keep a
separation between the different types of test though, especially
considering how big the unit tests may become. Then relax can present
a report of each category, and only pass if all categories pass. It
will allow more categories of tests to be added separately - for
example installation tests, compilation tests, etc.
Two other questions
1. do you use the '_'function_name convention for private functions
Currently relax doesn't have private functions - I don't see a need
for this, except of course for the eventual conversion of 'float.py'
into a separate Python module. I have used the 'self._xxxx()' style
only in the automatic diffusion tensor object creation code and this
only hides these functions by a search statement within the
'self.__repr__()' methods of the relax data structure
'self.relax.data'. I know Python fudges private functions using the
'self.__xxxx()' notation, but I have a feeling site-packages do this
differently, not through private objects but by other methods. I
could be wrong though.
2. I like the relax code style checker
(http://nmr-relax.com/scripts/code_validator). However, it seems to
produce an annoying number of false positives, is this something that is
being worked on? (it would be really nice to be able to run it and see
no complaints on conforming code, it would make life much more asy for
the devloper) Also I have a few pathological cases where it fails should
i foward those as some sort of bug report?
The repository of web pages have now been moved into the subversion
repository. It's in the directory 'website' viewable at
http://svn.gna.org/viewcvs/relax/. This 'code_validator' script was
an old script I used for cleaning up relax source code that I have
slightly extended. It has always reported many false positives but if
you would like to improve this script to avoid these false results, go
for it! There's probably not much use creating a bug report. Also,
each thing it checks has different levels of importance. Some parts
are essential, others are cosmetic, and one or two are highly
superficial. I am eventually planning on categorising these checks.
Cheers,
Edward
Re: r2860 - /branches/test_suite/test_suite/unit_tests/unit_test_runner.py