From: Karol L. <kar...@kn...> - 2007-04-26 10:27:49
|
On Thursday 26 April 2007 08:04, Noel wrote: > Do any of our parsers set the attribute, but then delattr it? Might be > worth checking. Also, it would be possible to check the types of > attributes at this point, using an assert statement. I can look into > this. langner@slim:~/tmp/python/cclib/trunk/src/cclib/parser$ grep "delattr" * logfileparser.py: delattr(self, attr) The actual parsing no - the only place it is used is in LogFile.clean(). What do you mean by "checking the types of attributes at this point"? Do you mean checking that the value is the correct type inside LogFile.__setattr()? > > 5) Numeric->numpy transition. This isn't strictly related to the > > refactoring, but it might as well be done at once. The related changes > > seem to be trivial. > > Can we leave this until all current tests pass after the first refactoring. Of course. I added it, becuase hopefully the refactoring will not break any tests. - Karol -- written by Karol Langner Thu Apr 26 14:26:08 CEST 2007 |
From: Noel O'B. <bao...@gm...> - 2007-04-26 10:57:23
|
On 26/04/07, Karol Langner <kar...@kn...> wrote: > On Thursday 26 April 2007 08:04, Noel wrote: > > Do any of our parsers set the attribute, but then delattr it? Might be > > worth checking. Also, it would be possible to check the types of > > attributes at this point, using an assert statement. I can look into > > this. > > langner@slim:~/tmp/python/cclib/trunk/src/cclib/parser$ grep "delattr" * > logfileparser.py: delattr(self, attr) > > The actual parsing no - the only place it is used is in LogFile.clean(). > > What do you mean by "checking the types of attributes at this point"? Do you > mean checking that the value is the correct type inside LogFile.__setattr()? E.g. verify that mocoeffs is a list of 1 or 2 numeric arrays of rank 2 or whatever. It wouldn't add much overhead to the program, and would act as a sanity check. > > > 5) Numeric->numpy transition. This isn't strictly related to the > > > refactoring, but it might as well be done at once. The related changes > > > seem to be trivial. > > > > Can we leave this until all current tests pass after the first refactoring. > > Of course. I added it, becuase hopefully the refactoring will not break any > tests. > > - Karol > > -- > written by Karol Langner > Thu Apr 26 14:26:08 CEST 2007 > > ------------------------------------------------------------------------- > This SF.net email is sponsored by DB2 Express > Download DB2 Express C - the FREE version of DB2 express and take > control of your XML. No limits. Just data. Click to get it now. > http://sourceforge.net/powerbar/db2/ > _______________________________________________ > cclib-devel mailing list > ccl...@li... > https://lists.sourceforge.net/lists/listinfo/cclib-devel > |
From: Karol L. <kar...@kn...> - 2007-04-26 11:23:26
|
On Thursday 26 April 2007 12:57, Noel O'Boyle wrote: > On 26/04/07, Karol Langner <kar...@kn...> wrote: > > On Thursday 26 April 2007 08:04, Noel wrote: > > > Do any of our parsers set the attribute, but then delattr it? Might be > > > worth checking. Also, it would be possible to check the types of > > > attributes at this point, using an assert statement. I can look into > > > this. > > > > langner@slim:~/tmp/python/cclib/trunk/src/cclib/parser$ grep "delattr" * > > logfileparser.py: delattr(self, attr) > > > > The actual parsing no - the only place it is used is in LogFile.clean(). > > > > What do you mean by "checking the types of attributes at this point"? Do > > you mean checking that the value is the correct type inside > > LogFile.__setattr()? > > E.g. verify that mocoeffs is a list of 1 or 2 numeric arrays of rank 2 > or whatever. It wouldn't add much overhead to the program, and would > act as a sanity check. As you know, presently types are checked after parsing (I added the _attrtypes list today). This could be done additionally in __setattr__. Checking the contents of lists, though, is done only for lists that are supposed to contain arrays. The method __setattr__, however, is called only when a new value is assigned (using equivalence or whatever). So that would be a rather weak sanity check for list contents - if something is appended to the list, or one of the elements changed, the method is not called. - Karol -- written by Karol Langner Thu Apr 26 15:11:57 CEST 2007 |
From: Noel O'B. <bao...@gm...> - 2007-04-26 11:32:03
|
On 26/04/07, Karol Langner <kar...@kn...> wrote: > On Thursday 26 April 2007 12:57, Noel O'Boyle wrote: > > On 26/04/07, Karol Langner <kar...@kn...> wrote: > > > On Thursday 26 April 2007 08:04, Noel wrote: > > > > Do any of our parsers set the attribute, but then delattr it? Might be > > > > worth checking. Also, it would be possible to check the types of > > > > attributes at this point, using an assert statement. I can look into > > > > this. > > > > > > langner@slim:~/tmp/python/cclib/trunk/src/cclib/parser$ grep "delattr" * > > > logfileparser.py: delattr(self, attr) > > > > > > The actual parsing no - the only place it is used is in LogFile.clean(). > > > > > > What do you mean by "checking the types of attributes at this point"? Do > > > you mean checking that the value is the correct type inside > > > LogFile.__setattr()? > > > > E.g. verify that mocoeffs is a list of 1 or 2 numeric arrays of rank 2 > > or whatever. It wouldn't add much overhead to the program, and would > > act as a sanity check. > > As you know, presently types are checked after parsing (I added the _attrtypes > list today). This could be done additionally in __setattr__. Checking the > contents of lists, though, is done only for lists that are supposed to > contain arrays. The method __setattr__, however, is called only when a new > value is assigned (using equivalence or whatever). So that would be a rather > weak sanity check for list contents - if something is appended to the list, > or one of the elements changed, the method is not called. > Sorry - I forget that it was already being checked after parsing. > - Karol > > -- > written by Karol Langner > Thu Apr 26 15:11:57 CEST 2007 > > ------------------------------------------------------------------------- > This SF.net email is sponsored by DB2 Express > Download DB2 Express C - the FREE version of DB2 express and take > control of your XML. No limits. Just data. Click to get it now. > http://sourceforge.net/powerbar/db2/ > _______________________________________________ > cclib-devel mailing list > ccl...@li... > https://lists.sourceforge.net/lists/listinfo/cclib-devel > |
From: Karol L. <kar...@kn...> - 2007-04-26 13:26:34
|
Hi, With revision 614, I have basically completed the refactoring changes I planned. All tests still pass, two ADF regression tests fail, but this was true before my commits, if I remember correctly. I'm probably not going to do anything else today, to let the changes "sink in". I might add a few more minor things tommorow related with the refactoring, though. This would be good moment to discuss the possibility of using a dictionary of functions instead of the extract() method, or other further ideas. My main worry is where all the functions would be defined. - Karol -- written by Karol Langner Thu Apr 26 17:17:09 CEST 2007 |