Camelia, the Perl 6 bug

IRC log for #cdk, 2011-11-02

| Channels | #cdk index | Today | | Search | Google Search | Plain-Text | summary

All times shown according to UTC.

Time Nick Message
04:51 sneumann_ joined #cdk
05:07 egonw joined #cdk
05:52 jbrefort joined #cdk
06:07 egonw joined #cdk
06:25 sneumann_ joined #cdk
06:45 egonw_ joined #cdk
06:57 CIA-38 cdk: Egon Willighagen master * rd0d1b14 / (8 files in 8 dirs): Merged cdk-1.4.x (+10 more commits...) - http://git.io/EgMbuA
07:26 jonalv joined #cdk
07:37 egonw_ joined #cdk
08:02 Gpox joined #cdk
10:32 egonw joined #cdk
10:33 Gpox joined #cdk
10:33 mgerlich joined #cdk
10:55 CIA-81 joined #cdk
12:58 CIA-81 cdk: Egon Willighagen master * r3eaeaa7 / src/META-INF/smarts.cdkdepends : Removed non-existing dependency - http://git.io/aboiHg
13:22 jonalv egonw: okey, new pull request for you :)
13:24 egonw I see one new commit, but only three older
13:26 egonw is that correct?
13:39 egonw and the only change I see is the getSubstructure(int) method (you did not like Rajarshi's suggestion?)
13:39 egonw but no JavaDoc fix...
13:39 egonw jonalv: there seem commits missing...
13:40 jonalv egonw: oh how come?
13:40 egonw dunno... you tell me why those commits are not there
13:41 jonalv egonw: you tell me what commits you are missing I will try to tell you why :)
13:41 egonw well, you patch replied to only one comment
13:41 jonalv my patch reply?
13:42 jonalv uhm
13:42 jonalv sounds strange
13:42 jonalv egonw: are you looking at: https://github.com/cdk/cdk/pull/4
13:43 egonw sort of
13:43 egonw but those four patche, yes
13:43 egonw you had 4 in the previous pulll
13:43 egonw one got lost?
13:43 jonalv I merged two
13:43 jonalv and added one
13:44 egonw ah...
13:44 egonw the commit dates were old...
13:44 egonw that got me confused
13:44 jonalv well rebase changes commits dates no?
13:45 egonw apaprently not
13:45 egonw OMG...
13:45 egonw one of my comments got misplaced!
13:45 egonw about the brackets!
13:45 egonw https://github.com/cdk/cdk/pull/3
13:46 egonw third comment
13:46 jonalv no wirries I am not touching theose SMARTS anyways
13:46 egonw that's the problem!
13:46 egonw the comments were *not* about the SMARTS!
13:46 jonalv I don't know enough to touch them
13:46 jonalv oh?
13:48 egonw this may be caused by you using the same branch
13:48 egonw rewriting history...
13:48 egonw fucking up the comments ...
13:48 egonw well, their positions anyway
13:49 jonalv well I am sorry I thought I was supposed to rewrite histoury
13:49 egonw the brackets were not about the SMARTSs, but about something else...
13:49 jonalv it's not so easy for a bear of very little brain
13:49 egonw you had something like @TestMethod("foo()") which should be just "foo"
13:50 jonalv egonw: okey, that will have gone away after I changed to extening the SubstructureFingerprinter anyway
13:50 egonw indeed
13:50 egonw it seems you read the comment after that or so...
13:51 egonw because you're reply is clearly about me commenting on the SMARTS, which is after GitHub messed up the comment placement :(
13:51 jonalv okey
13:51 egonw OK, I'm good
13:51 jonalv but no worries casue it doesn't apply in the current future cause I rewrote histopury :)
13:51 jonalv *history
13:52 egonw that leaves Rajarshi's comment on why not make getSubstructure(int) part of the substructure FP interface
13:52 jonalv well, that is easy
13:52 jonalv there is no such interface
13:52 jonalv I made it part of the SubstructureFingerprint class
13:52 egonw ah, you did
13:52 egonw ah, another request then :)
13:53 egonw well....
13:53 egonw 1. great, because then it automatically applies to the other extensions too
13:53 jonalv nope I think it's fine as it is for the moment :)
13:53 egonw 2. but it's missing a unit test now
13:53 egonw I'll comment :)
13:54 jonalv egonw: oh darn you saw that. I didn't know where to put that test
13:54 jonalv :)
13:55 egonw well, there is nothing to go into AbstractSubstructureFPTest here...
13:55 jonalv besides I thought it was rather safe. Not much that could go wrong with that...
13:55 jonalv is there usch a class?
13:55 egonw these unit test methods need to go in the various impls
13:55 egonw as they depend on that particular list of SMARTS
13:55 egonw no, not at this moment...
13:55 egonw there is AbstractFingerprinterTest
13:55 egonw but it should not go there...
13:56 jonalv well I disagree, the only thing we need to test is the lookup. Which if it works should work for all of them
13:56 egonw how would you test that then?
13:56 egonw how do you know what the proper int is?
13:56 jonalv besides the lokup is just an array lookup by index and I don't think we need to test that
13:56 egonw OK, fair
13:56 egonw test these things:
13:57 egonw return some non-null, lognerThanOneLength string for these ints:
13:57 egonw 1 and sizeOfFP
13:57 egonw (or starting at zero?)
13:57 egonw OK...
13:57 egonw where it should go:
13:57 egonw public class AbstractSubstructureFingerprinterTest extends AbstractFingerprinterTest { }
13:57 egonw that is, create a new class :)
13:58 egonw ummm: public abstract class ...
13:58 egonw with nothing more than the above unit test
13:58 egonw and have the KRFPTest class extend AbstractSubstructureFingerprinterTest isntead of AbstractFingerprinterTest
13:59 jonalv uhm
13:59 jonalv all this just to test a array index lookup?
14:01 jonalv okey...
14:01 egonw yeah, that's the thing with API changes :)
14:01 jonalv itäs just that this test is meaningless in my humble opinion
14:03 egonw the funny thing is that I have seen many meaningless unit tests highlight bugs in the code ...
14:05 jonalv egonw: but there is a SubstructureFingerprintTest so makes sense to use that I think
14:05 egonw yes, good call
14:05 egonw yes, I'm happy with that
14:13 jonalv egonw: okey, done, one more patch added :)
14:16 egonw okey... I think this will give some minor remaining issues...
14:17 egonw I'll report...
14:17 egonw applying now... almost there
14:17 egonw have you tried this on the command line:
14:17 egonw ant -Dmodule=fingerprint qa-module
14:18 egonw qa = quality assurance
14:18 jonalv trying now
14:18 jonalv BUILD SUCCESSFUL
14:18 jonalv is that good?
14:19 egonw a good start
14:19 egonw check the reports created in reports/
14:19 jonalv that's almost 50 files
14:19 jonalv what am I looking for?
14:20 egonw the files about the fingerprint module
14:20 egonw try this isntead:
14:20 egonw ant -Dmodule=fingerprint clean qa-module
14:20 jonalv now I get BUILD FAILED
14:21 egonw oh...
14:21 jonalv build/fingerprint.javafiles not found
14:21 egonw umm
14:21 egonw ant -Dmodule=fingerprint clean dist-all test-dist-allqa-module
14:21 egonw ant -Dmodule=fingerprint clean dist-all test-dist-all qa-module
14:21 egonw or maybe just:
14:21 egonw ant -Dmodule=fingerprint clean runDoclet qa-module
14:22 jonalv java.lang.ClassNotFoundException: org.openscience.cdk.modulesuites.MfingerprintTests
14:23 jonalv this stuff scares me
14:24 egonw ah, right
14:24 egonw so:
14:24 egonw ant -Dmodule=fingerprint clean dist-all test-dist-all qa-module
14:24 jonalv BUILD FAILED
14:25 egonw how?
14:25 jonalv that's sort of what I am wondering too
14:26 egonw but you got output
14:27 jonalv egonw: http://pastebin.com/nmYZ4PMU
14:30 jonalv egonw: do you understand that stuff?
14:30 egonw did you run:
14:30 egonw ant -Dmodule=fingerprint clean dist-all test-dist-all qa-module
14:30 egonw mmm...
14:31 egonw OK, maybe do it in two steps:
14:31 egonw ant clean dist-all test-dist-all
14:31 egonw ant -Dmodule=fingerprint jarTestData qa-module
14:31 jonalv BUILD FAILED
14:31 jonalv Target "jarTestData" does not exist in the project "CDK".
14:32 egonw small d
14:32 egonw ant -Dmodule=fingerprint jarTestdata qa-module
14:32 jonalv okey
14:37 jonalv egonw: if that was the first step what is the second step?
14:38 egonw [11/02/11 15:31] <egonw> ant clean dist-all test-dist-all
14:38 egonw [11/02/11 15:31] <egonw> ant -Dmodule=fingerprint jarTestData qa-module
14:38 egonw that's two steps?
14:39 jonalv uhm okey, the first step gives me BUILD FAILED
14:40 egonw then you have a compile failure
14:40 jonalv apparently it's not finding the SilentChemObjectBuilder
14:40 egonw ah
14:40 egonw edit src/META-INF/fingerprint.cdkdepends
14:40 egonw and add the cdk-silent.jar
14:43 jonalv egonw: it still complains about that
14:44 jonalv package org.openscience.cdk.silent does not exist
14:44 egonw paste me the diff of src/META-INF
14:45 egonw git diff -- src/META-INF
14:45 jonalv http://pastebin.com/E5JFF08d
14:46 egonw do you have a dist/jar/cdk-silent.jar ?
14:46 egonw I checked the build.xml and silent is compiled before fingerprint
14:46 egonw so it should be there
14:47 jonalv yea it's there
14:48 egonw oh carp
14:48 egonw my bad
14:49 egonw remove it from fingerprint.cdkdepends
14:49 egonw add it to:
14:49 egonw test-fingerprint.cdkdepends
14:49 egonw because it's the test-fingerprint module that needs it
14:49 egonw not the fingerprint module
14:53 jbrefort joined #cdk
14:53 jonalv okey that seemed to work
14:53 jonalv egonw: how do I know what to worry about in that txt file now?
14:54 egonw what txt file?
14:54 egonw those reports?
14:54 jonalv result-fingerprint.txt
14:54 jonalv yea
14:54 egonw check anything that is about your new classes
14:54 jonalv it doesn't seem to be anything related to my changes
14:55 egonw that sounds good
14:55 egonw make sure to check the unit tests...
14:55 jonalv in that file at least
14:55 egonw and check with nightly that no existing unit tests are failing that were not before your changes
14:55 jonalv uhm I don't understand how to do that
14:57 jonalv isn't the result-fingerprinbt.txt file the unit tests_
14:57 jonalv ?
14:57 egonw yeah, could be :)
14:57 egonw not so good at remembering file names
14:58 egonw there should also be files for the PMD tests, and JavaDoc tests
14:58 jonalv ojdcheck and pmd
14:58 jonalv well pmd is xml
14:58 jonalv unreadable
14:58 egonw yes, and yes
14:58 egonw and no
14:59 egonw xsltproc ./pmd/wz-pmd-report.xslt report/$unreadableFoo.xml
15:00 jonalv if this is what it takes to make a small change in CDK I am suprised you get anything done at all
15:00 egonw then open the resulting HTML in a browser
15:00 egonw well, it's when you make a patch, or later
15:00 egonw the fact that authors now do this, means that I don't
15:00 egonw that way, I can actually get something done in the CDK
15:01 jonalv warning: failed to load external entity "./pmd/wz-pmd-report.xslt"
15:01 jonalv cannot parse ./pmd/wz-pmd-report.xslt
15:01 egonw oh, weird
15:01 egonw well, just grep the content for anything with Klethora
15:02 jonalv no match
15:02 jonalv what\s pmd-migrating and pmd-unused_
15:05 jonalv egonw: but then I think it's fine, or is there something else that I need to do?
15:05 egonw nope
15:05 jonalv just gonna commit the last chagne
15:07 jonalv egonw: done
15:07 egonw thanx
15:07 egonw in a conf call now, but will apply soon
15:08 egonw thanx for your effort!
15:08 jonalv oki
15:09 jonalv egonw: this makes me realise that it would be good if we could be in the same location when it comes to doing this for my API changing branch
15:10 egonw :)
15:10 jonalv it also makes me wonder if it is worth all the hassle... :(
15:10 egonw well, you learned a lot of tricks now :)
15:10 * jonalv is not sure
15:10 jonalv might need to repeat this a couple of times...
15:10 egonw probably (I had too)
15:10 egonw and I still need to look up stuff now and then
15:58 CIA-81 cdk: Egon Willighagen master * rc3040e8 / (3 files):
15:58 CIA-81 cdk: More use of interfaces rather than implementations for the smiles module
15:58 CIA-81 cdk: Signed-off-by: Rajarshi Guha <rajarshi.guha@gmail.com> - http://git.io/3ybVOg
16:57 CIA-81 cdk: jonalv cdk-1.4.x * r135edeb / src/META-INF/test-fingerprint.cdkdepends :
16:57 CIA-81 cdk: fixed dependency for fingerprint tests
16:57 CIA-81 cdk: Signed-off-by: Egon Willighagen <egonw@users.sourceforge.net> (+5 more commits...) - http://git.io/mabFqg
18:38 sneumann_ joined #cdk
20:13 egonw joined #cdk

| Channels | #cdk index | Today | | Search | Google Search | Plain-Text | summary