From: E.L. W. <eg...@sc...> - 2002-09-19 11:56:00
|
Hi all, In the last two weeks I encountered smaller issues in CDK, like duplicate code, documentation issues, wrong package etc... They may not all be bugs in the code, but often lack of documentation too. I already forgot a number of them. I do not feel much for filing them all as bugs on SF, thus, here they are (up for discussion): 1. Much use of template JavaDoc. I am removing those at this moment as it interferce with calculation JavaDoc statistics. 2. IsomorphChecker does only check the adjacency matrix, not the connection matrix. Is this intended? 3. SwissArmyKnife has a method generateElementFormula() which should be MFAnalyser. 4. Empty @since lines... should be removed (see 1.) 5. PathTools should be in the package cdk.graph and related to it 6. Package cdk.graphinvariant should be cdk.graph.invariant 7. Documentation is missing about the differences between AtomTypeFactory and IsotopeFactory. 8. SmilesParser possibly uses an Exception from compchem. 9. The class cdk.smiles.AromaticityCalculator must be in the package cdk.aromaticity 10. IsomorphismTester mentions it is "too simplistic", but it does not state what the limitations are... 11. CanonicalLabeler and MorganNumbersTools might be placed in a new package for classes calculation Atom numberers? 12. Should HOSECodeGenerator be moved into a new package fo classes that calculate Atom descriptors? 13. CanonicalLabeler contains @roseuid tags in JavaDoc. If no one uses them, could they be removed? It also contains rudimentary "@return void" clauses that can be removed too, as well as those empty @exception tags. 14. FourierGridBasis is not implemented correctly. 15. FoureirGridBasis contains physical properties that belong elsewhere, like mass of electron and proton. I also saw they Bohr radius somewhere that belongs that class 'elsewhere' too. More will probably follow. Please email/file such smaller bug/issues too. I do not expect them to be solved instantianously, or by people other than myself... this email is mostly for documentational purposes... regards, Egon |
From: Christoph S. <ste...@ic...> - 2002-09-19 12:42:14
|
Just a few comments for the cases that belong to me.... E.L. Willighagen wrote: > 2. IsomorphChecker does only check the adjacency matrix, not the > connection matrix. Is this intended? This is because it relies on calculating Morgan numbers in the classical form. This form does not take into account bond orders. > 8. SmilesParser possibly uses an Exception from compchem. We don't have any compchem.jar so it cannot rely on this. We can remove this line, if there is any. > 10. IsomorphismTester mentions it is "too simplistic", but it does not state > what the limitations are... Well, it calculates MorganNumbers for two graphs and compares them for each atom and its direct neighbors. As far as I know this is not sufficient but works in 99%. Also takes into account the element symbols. > 11. CanonicalLabeler and MorganNumbersTools might be placed in a new > package for classes calculation Atom numberers? They should go/be in graph.invariant > 12. Should HOSECodeGenerator be moved into a new package fo classes > that calculate Atom descriptors? We could do this, but I wonder if we will get so many more classes that produce atom centered codes. Maybe we will... -- Dr. Christoph Steinbeck (http://www.ice.mpg.de/departments/ChemInf) MPI of Chemical Ecology, Winzerlaer Str. 10, Beutenberg Campus, 07745 Jena, Germany Tel: +49(0)3641 571263 - Fax: +49(0)3641 571202 What is man but that lofty spirit - that sense of enterprise. ... Kirk, "I, Mudd," stardate 4513.3.. |
From: E.L. W. <eg...@sc...> - 2002-09-20 11:43:03
|
On Thursday 19 September 2002 13:55, E.L. Willighagen wrote: > 1. Much use of template JavaDoc. I am removing those at this moment as > it interferce with calculation JavaDoc statistics. Done. > 4. Empty @since lines... should be removed (see 1.) Done. > 8. SmilesParser possibly uses an Exception from compchem. Done. Egon |
From: E.L. W. <eg...@sc...> - 2002-09-27 13:34:23
|
Here are the resolutions. On Thursday 19 September 2002 13:55, E.L. Willighagen wrote: > 1. Much use of template JavaDoc. I am removing those at this moment as > it interferce with calculation JavaDoc statistics. Done. > 2. IsomorphChecker does only check the adjacency matrix, not the > connection matrix. Is this intended? Yes. Comment added. > 3. SwissArmyKnife has a method generateElementFormula() which should > be MFAnalyser. Done. > 4. Empty @since lines... should be removed (see 1.) Done. > 5. PathTools should be in the package cdk.graph > and related to it > 6. Package cdk.graphinvariant should be cdk.graph.invariant Done. > 7. Documentation is missing about the differences between AtomTypeFactory > and IsotopeFactory. > > 8. SmilesParser possibly uses an Exception from compchem. Removed. > 9. The class cdk.smiles.AromaticityCalculator must be in the package > cdk.aromaticity Done. > 10. IsomorphismTester mentions it is "too simplistic", but it does not > state what the limitations are... See 2. > 11. CanonicalLabeler and MorganNumbersTools might be placed in a new > package for classes calculation Atom numberers? Done > 12. Should HOSECodeGenerator be moved into a new package fo classes > that calculate Atom descriptors? Maybe in the future. > 13. CanonicalLabeler contains @roseuid tags in JavaDoc. If no one uses > them, could they be removed? It also contains rudimentary "@return void" > clauses that can be removed too, as well as those empty @exception tags. Removed. > 14. FourierGridBasis is not implemented correctly. Not able to fix that myself. Posted on SF. > 15. FoureirGridBasis contains physical properties that belong elsewhere, > like mass of electron and proton. I also saw they Bohr radius somewhere > that belongs that class 'elsewhere' too. Add class PhysicalConstants. That's al... Egon |