From: SourceForge.net <no...@so...> - 2010-08-14 13:12:10
|
Patches item #3040138, was opened at 2010-08-05 18:32 Message generated for change (Comment added) made by egonw You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3040138&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: Needs Review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Egon Willighagen (egonw) Assigned to: Nobody/Anonymous (nobody) Summary: Code clean up in the DeduceBondSystem tool Initial Comment: Four patches that clean up the code in the DeduceBondSystemTool: 0001 is a request from Nina to be able to set a custom AllRingFinder instance, e.g. with a time-out 0002 is a performance fix... the AllRingFinder has the option for a while now to not find rings larger than some threshold. Now, the DeduceBondSystemTool does not deal with rings with more than seven atoms anyway, so there's our threshold... should speed up the performance on molecules with large ring systems 0003 missing unit tests, as reported by our coverage testing 0004 more QA fixes: missing @TestMethod annotation, JavaDoc, etc. Note that the last patch also sneaks in a fix that when setInterrupted(true) is called and performed, that the boolean is reset, to allow further use of the class (as the use case in the unit test class does). ---------------------------------------------------------------------- >Comment By: Egon Willighagen (egonw) Date: 2010-08-14 15:12 Message: Yeah, that's an existing failing unit test: http://pele.farmbio.uu.se/nightly/test/result-smiles.html It is indeed not fixed by the code cleanup. ---------------------------------------------------------------------- Comment By: gilleain maclean torrance (gilleain) Date: 2010-08-14 15:04 Message: Looks alright. I guess the main change is the new line: IRingSet rs = allRingsFinder.findAllRings(m,7); however, the test xtestQuinone fails with : java.lang.AssertionError: expected:<SINGLE> but was:<DOUBLE> org.openscience.cdk.smiles.DeduceBondSystemToolTest.xtestQuinone(DeduceBondSystemToolTest.java:164) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3040138&group_id=20024 |