[672c44da] | 1 | - Change SingleLoader class name to Loader. There's no reason to add confusion by making the name more complicated. |
---|
| 2 | - Remove all print statements such as: print "Hello", or print "in loader". If you need to log something, use python's logging module. |
---|
| 3 | - The following error condition is not dealt with properly: |
---|
| 4 | If the used calls Loader.load(path) for a file that has an unknown extension, an exception should be raise. |
---|
| 5 | As it is load() will call lookup(), which will simply print "Unknown file type '%s'"%ext. |
---|
| 6 | Modules using that function will then fail to recognize the error condition. |
---|
| 7 | |
---|
| 8 | - Look at all the try-except blocks and deal with error conditions. |
---|
| 9 | - Golden rule of error handling: if you catch an exception and all you do is calling pass or print, you have not dealt with the error properly. |
---|
| 10 | If you think you did, then you should definitely explain why in a comment so that developers looking at the code don't |
---|
| 11 | think that it's a bug. |
---|
| 12 | |
---|
| 13 | - Document all classes and methods, and all inputs and outputs. |
---|
| 14 | - Where are the comments for log() going? That should be documented in the code. |
---|
[1b0b3ca] | 15 | [changed]- Why is the description of the DataLoader module in setup.py "Python module for fitting"? |
---|
[672c44da] | 16 | - The return object of a load seems to be inconsistent and depend only on the particular reader. This will cause problems since |
---|
| 17 | the calling modules will be unable to know what to expect. An interface should be defined. |
---|
[f8de7de] | 18 | |
---|
| 19 | * Functional requirements |
---|
| 20 | - Multiple readers might be able to read files with a given extension. (For instance, .txt can |
---|
| 21 | be read by the 2-column reader and the 3-column reader.) If the first one raises an exception, |
---|
| 22 | the second should be tried. This means that all exceptions raised by a reader should be caught. |
---|
| 23 | |
---|
| 24 | - All data loaded from a file should be stored in a common class object. |
---|
| 25 | |
---|
[8bd8ea4] | 26 | - Avoid use of __setitem__ from outside a class. |
---|
| 27 | - Bug: Loading blah.my.txt won't work. Is fixed in reflectivity registry.py |
---|
| 28 | - Bug: The extension recognition is case sensitive and shouldn't be. |
---|
| 29 | - BUG: if a reader raises an exception other than ValueError, the loader doesn't catch it. |
---|
| 30 | |
---|
[f8de7de] | 31 | * Error conditions |
---|
| 32 | - Trying to load a file with an unknown extension should raise a RuntimeException. |
---|
| 33 | - Trying to load a corrupted file with a know file extension should raise a RuntimeException. |
---|