| Time |
S |
Nick |
Message |
| 03:56 |
|
|
azeem joined #cdk |
| 05:35 |
|
|
egonw joined #cdk |
| 06:13 |
|
|
jbrefort joined #cdk |
| 07:37 |
|
|
bag_ joined #cdk |
| 10:58 |
|
|
maclean joined #cdk |
| 10:59 |
|
|
bag_ joined #cdk |
| 11:03 |
|
|
egonw joined #cdk |
| 11:27 |
|
maclean |
ahhhh. git revert -n HEAD. ahhh. |
| 11:40 |
|
maclean |
egonw : reviewed/signoff-ed on a javadoc patch (with a tiny fix) - what now? |
| 11:40 |
|
egonw |
super! |
| 11:40 |
|
egonw |
git format-patch -1 |
| 11:40 |
|
egonw |
or |
| 11:40 |
|
egonw |
git format-patch -2 |
| 11:41 |
|
egonw |
and attached those to the patch report |
| 11:41 |
|
maclean |
ok |
| 11:52 |
|
maclean |
er... egonw : I don't seem to have permission to add stuff to patches. Or, at least not those ones... |
| 11:52 |
|
egonw |
mom |
| 11:54 |
|
egonw |
ok, can you try again? |
| 11:55 |
|
maclean |
aha. works now, takk. |
| 12:23 |
|
CIA-50 |
cdk: Egon Willighagen cdk-1.2.x * r38873dc / src/main/org/openscience/cdk/tools/CDKHydrogenAdder.java : |
| 12:23 |
|
CIA-50 |
cdk: Updated the JavaDoc for the atoms() Iterable API change (fixes #3034824) |
| 12:23 |
|
CIA-50 |
cdk: Signed-off-by: maclean <gilleain.torrance gmail.com> - http://bit.ly/b1KbZr |
| 12:24 |
|
egonw |
maclean: please use the 'Group' field next time to mark the patch as 'Accepted' |
| 12:24 |
|
maclean |
ok |
| 12:24 |
|
egonw |
otherwise, thanx for the review! |
| 12:24 |
|
egonw |
very, very much appreciated |
| 12:25 |
|
* maclean |
is now On branch patch-3043782 |
| 12:26 |
|
egonw |
got to reboot my KDE session |
| 12:26 |
|
egonw |
brb |
| 12:28 |
|
|
egonw joined #cdk |
| 12:29 |
|
egonw |
and I'm back |
| 13:12 |
|
egonw |
maclean: oh, and if you need the patch author to make changes, please mark it as 'Needs Revisions" |
| 13:12 |
|
maclean |
ah, ok. |
| 13:12 |
|
egonw |
replied to your comment on the DeduceBondSystemTool patch |
| 13:12 |
|
egonw |
that failing unit test is not new |
| 13:12 |
|
egonw |
see: http://pele.farmbio.uu.se/nigh[…]esult-smiles.html |
| 13:12 |
|
zarah |
egonw's link is also http://tinyurl.com/2dzmlex |
| 13:12 |
|
egonw |
but not fixed either |
| 13:14 |
|
maclean |
I suspect that it is testing for one of the possible bond systems, and not another. |
| 13:17 |
|
egonw |
good point |
| 13:17 |
|
egonw |
I have not checked that |
| 13:21 |
|
maclean |
It's assuming C1(=O)C=CC(=O)C=C1 and getting C1(O)=CC=C(O)C=C1 |
| 13:23 |
|
egonw |
you could update the test to see if the ring has the right number of DOUBLE and SINGLE bonds instead |
| 13:24 |
|
egonw |
and that the alternate or so |
| 13:24 |
|
maclean |
Hmm. Actually these two forms have different numbers of single/double bonds. |
| 13:25 |
|
egonw |
oh... ah indeed... |
| 13:25 |
|
egonw |
yeah, this is tautomerism... |
| 13:25 |
|
egonw |
correct |
| 13:25 |
|
egonw |
well, then it should be the first |
| 13:25 |
|
maclean |
I guess the form with singly-bonded oxygens has charges on the oxygens? |
| 13:25 |
|
egonw |
this is the well known example... |
| 13:25 |
|
egonw |
this compound is not aromatic |
| 13:26 |
|
maclean |
Ah |
| 13:26 |
|
egonw |
benzoquinone |
| 13:26 |
|
maclean |
1,4- |
| 13:26 |
|
egonw |
http://pubchem.ncbi.nlm.nih.go[…]d=4650&loc=ec_rcs |
| 13:26 |
|
zarah |
egonw's link is also http://tinyurl.com/3xl7m8t |
| 13:30 |
|
maclean |
So, this test relates to my question of what the DeduceBondSystemTool is actually trying to do... |
| 13:31 |
|
egonw |
given hybridization information, resolve where the double bonds should be |
| 13:31 |
|
egonw |
basically comes down to: |
| 13:31 |
|
egonw |
for linear systems, start at one end and alternate |
| 13:31 |
|
egonw |
for ring systems, make a arbitrary cut |
| 13:31 |
|
egonw |
and mix the lot |
| 13:35 |
|
|
maclean_ joined #cdk |
| 13:36 |
|
maclean_ |
Well, from a code point of view, it will take 1,4-BZQ and generate 3 bonds for the benzene ring. |
| 13:36 |
|
maclean_ |
There's really no possibility for the code to pass the test - it's not really a bug. |
| 13:41 |
|
maclean |
Actually, no that's not true. It generates all sorts of possibilities: |
| 13:41 |
|
maclean |
[[[0-1, 2-3, 4-5], [1-2, 3-4, 5-0], [0-1, 2-3], [0-1, 4-5], [1-2, 3-4], [1-2, 0-5], [2-3, 4-5], [0-5, 3-4], [0-1, 3-4], [1-2, 4-5], [2-3, 0-5], [0-1], [1-2], [2-3], [3-4], [4-5], [5-0], []]] |
| 13:46 |
|
maclean |
Ah, ok, which is all orbits of the colorings of the line graph :) |
| 13:46 |
|
maclean |
So, [1-2, 4-5] is the one that is wanted. |
| 13:48 |
|
|
maclean_ joined #cdk |
| 13:53 |
|
egonw |
maclean++ |
| 13:54 |
|
maclean |
Well, this doesn't really fix it... |
| 14:11 |
|
egonw |
btw, the x in front of the method name indicates that under a JUnit3 world it was not actually run... |
| 14:11 |
|
egonw |
perhaps the test was flawed for a very long time already... |
| 14:11 |
|
egonw |
then again... someone wrote it for some reason at some point, I guess... |
| 14:15 |
|
maclean |
ah, I see. |
| 16:16 |
|
|
jbrefort joined #cdk |
| 17:08 |
|
CIA-50 |
cdk: maclean master * r7959947 / .classpath : |
| 17:08 |
|
CIA-50 |
cdk: Exporting the signatures jar |
| 17:08 |
|
CIA-50 |
cdk: Signed-off-by: Egon Willighagen <egonwusers.sourceforge.net> - http://bit.ly/cejKPk |
| 17:08 |
|
maclean |
ta |
| 17:08 |
|
egonw |
yeah, some patches are really easy to review :) |
| 17:09 |
|
maclean |
well, true. |
| 17:14 |
|
maclean |
egonw: What do you think of : "interface ChemicalFilter<T extends IChemObject> { public boolean accept(T); }" |
| 17:14 |
|
egonw |
sounds like something olas would be happy with |
| 17:14 |
|
egonw |
any planned implementations? |
| 17:14 |
|
egonw |
prefiltering for QSAR? |
| 17:15 |
|
maclean |
For the multiple bond stuff. |
| 17:15 |
|
egonw |
ok |
| 17:15 |
|
egonw |
cdk.filter ? |
| 17:15 |
|
egonw |
ok, going to have dinner now |
| 17:15 |
|
egonw |
bbl |
| 17:15 |
|
maclean |
You generate possibilities, and filter out those that don't work, chemically. |
| 17:15 |
|
maclean |
ok, cu. |
| 17:38 |
|
|
egonw joined #cdk |
| 18:08 |
|
|
maclean joined #cdk |
| 18:28 |
|
maclean |
egonw : My blog post was wrong, I think. It doesn't generate B as the C-O bonds are not considered. |
| 18:29 |
|
egonw |
I was only slightly worried about that |
| 18:29 |
|
maclean |
:) |
| 18:30 |
|
maclean |
Well, I'm re-writing the DBST and the ring bonds are fine. |
| 18:30 |
|
egonw |
you will make very many people happy |
| 18:30 |
|
maclean |
But working out what other bonds should be considered is more tricky. |
| 18:31 |
|
egonw |
true |
| 18:32 |
|
maclean |
Isn't phenol an example of where the ring bonds delocalise with the C-O? |
| 18:32 |
|
maclean |
Because, bonds attached to the ring could be considered. Then there's the problem of bonds that connect rings... |
| 18:33 |
|
maclean |
Aargh. |
| 18:33 |
|
egonw |
yes, non-trivial |
| 18:33 |
|
egonw |
this is why I was thinking about SINGLE_OR_DOUBLE |
| 18:33 |
|
egonw |
set by the SMILES parser |
| 18:33 |
|
egonw |
because then one would know which atoms should be considered... |
| 18:33 |
|
egonw |
but that was correctly rejected |
| 18:34 |
|
egonw |
order = UNKNOWN |
| 18:34 |
|
egonw |
that would still work |
| 18:34 |
|
egonw |
then the DBST would only look at bonds with UNKNOWN bond order |
| 18:35 |
|
maclean |
Hmmm. That could be okay. |
| 18:40 |
|
egonw |
but that does require some hacking on the SMILESParser... |
| 18:40 |
|
egonw |
if you would consider it... |
| 18:40 |
|
egonw |
make sure to make that a separate patch, with proper tests etc... |
| 18:41 |
|
egonw |
I will consider doing it when I am back from Boston, but not before |
| 18:42 |
|
maclean |
Totally fine. |
| 18:42 |
|
maclean |
At the moment, it's all in a separate project, not even a cdk branch. |
| 18:56 |
|
egonw |
maclean: related to functional and unit tests... http://chem-bla-ics.blogspot.c[…]dencies-with.html |
| 18:56 |
|
zarah |
egonw's link is also http://tinyurl.com/38smwq3 |
| 18:57 |
|
maclean |
Hah : "I don't know how useful it is, but if you try it, please come back to tell us whether it was useful." |
| 18:57 |
|
maclean |
:D |
| 18:58 |
|
maclean |
This reminds me of a family member's tendency to take things from the fridge and say "I think this has gone off - will you try it and see?" |
| 19:01 |
|
egonw |
:) |
| 19:22 |
|
egonw |
maclean: http://www.fatvat.co.uk/2009/0[…]-and-clojure.html |
| 19:22 |
|
zarah |
egonw's link is also http://tinyurl.com/n7nqdr |
| 19:22 |
|
egonw |
should likely work with any Java program, not just clojure |
| 19:25 |
|
maclean |
hmmm. interesting. seems like it's already installed with my jdk. nice interface. |
| 19:26 |
|
egonw |
sudo apt-get install visualvm |
| 19:52 |
|
* maclean |
is off to get some food. bye |
| 20:10 |
|
|
bag_ joined #cdk |
| 20:23 |
|
|
slyrus_ joined #cdk |
