On 11 November 2010 15:45, Noel O'Boyle <baoille...@gmail.com> wrote:
> ---------- Forwarded message ----------
> From: Andrew Dalke <da...@dalkescientific.com>
> Date: 11 November 2010 11:28
> Subject: [Open Babel] Python test cases
>
> I spent a lot of time on trains yesterday, and test cases were
> the right mental level for me.
>
> Here's what I developed:
>
> http://dalkescientific.com/test_python.py
>
> They are meant so that people have a way to figure out how
> to call different functions, and they are meant to be fast,
> so they can be used as real unit tests. (The 66 test cases /
> ~400 tests) run in about 1.5 seconds on my laptop.)
>
>
> I don't know how you all want that hooked into the existing test
> framework, but I figured it was best to start with test cases.
If you could add it to the test framework that'd be great. You just
need to edit test/CMakeLists.txt.
> It also looks like I should get commit privs to OpenBabel.
Can you create a sourceforge a/c and email the name to Geoff asking
for SVN access?
> I noticed a number of minor problems when developing the tests.
> These are all marked with an "XXX" in the above code, although
> there are a few places marked with XXX which are either TODOs
> or places where I didn't understand the result when I wrote it
> but I believe I do now.
>
> 1) If I do something like
>
> mol = parse_smiles("CCO")
> mol.SetTitle("#1")
> conv.WriteFile(mol, tempdir("blah.sdf"))
>
> I get the message
>
> Warning in WriteMolecule No 2D or 3D coordinates exist.
> Any stereochemical information will be lost. To generate
> 2D or 3D coordinates use --gen2D or --gen3d.
>
> Since I'm not (directly) calling WriteMolecule and since my
> test cases don't support the --gen2D or --gen3d flags, this
> warning isn't that helpful in a library function.
This is by design (added recently). Up until recently, new users
thought that Open Babel didn't work, and complained to the mailing
list. I remember it confused me too when I first did a SMILES to SDF
conversion and it came up all zeros! This way we annoy you and me but
everyone else is happy.
> 2) ob.OBFingerprint.List("fingerprints")
>
> This writes to an output stream, and I can't figure out
> how to generate a string-based output stream in Python.
> However, this is not that important because there are
> other ways to access the same data. It does suggest that
> List() is not a useful library function.
Yes. We can remove it from the Python API. I've tried to ensure that
'most everything is accessible through the API, and haven't worried
too much about functions that don't work if the information is already
accessible.
> 3) Why does the fingerprint GetBit require a fingerprinter instance?
> That is, I have to go through a bound method
>
> x = ob.OBFingerprint.FindFingerprint("FP2")
> self.assertEquals(x.GetBit(v, 0), True)
>
> instead of using a static/global function
>
> self.assertEquals(ob.OBFingerprint.GetBit(v, 0), True)
?
> 4) Here's a test case I didn't expect. It seems that GetUMapList
> interacts with the results of GetMapList.
>
> def test_basic_match_behavior_which_I_did_not_expect(self):
> mol = parse_smiles("c1ccccc1O")
> pat = parse_smarts("c1ccccc1")
> pat.Match(mol)
> self.assertEquals(pat.NumMatches(), 12)
> results = pat.GetUMapList()
> self.assertEquals(pat.NumMatches(), 1)
> results = pat.GetMapList()
> # I really expected these to be 12.
> # It appears the UMapList does an in-place trim.
> # XXX Is that the right/expected behavior?
> self.assertEquals(pat.NumMatches(), 1)
> self.assertEquals(len(results), 1)
>
> pat.Match(mol)
> # Here they are 12
> results = pat.GetMapList()
> self.assertEquals(pat.NumMatches(), 12)
> results = pat.GetUMapList()
> self.assertEquals(pat.NumMatches(), 1)
> self.assertEquals(len(results), 1)
Looks unexpected to me. Maybe should file a bug.
> 5) The BeginMList/EndMList seems broken in Python. That is,
> I couldn't figure how how to use them.
All Begin/End methods do not work from Python. Many of them are
disabled from Python.
> 6) I included tests for iterating over atoms and bonds in
> a molecule, and for atoms and bonds connected to an atom. I
> expected I could also iterate over the atoms in a bond, but
> there is no OBBondAtomIter . While not important, it appeals
> to my sense of symmetry and it's very simple to write.
True, but will have to wait until next API change.
> 7) Why can't I do mol.AddBond(C, N, 3) where C and N are atoms?
> Instead, I need to do mol.AddBond(C.GetIdx(), N.GetIdx(), 3)
No reason I guess. Could cause problems where C and N are not members
of the molecule though.
> 8) Why are there two ways to add an atom? These are:
> atom = mol.NewAtom()
> -and-
> atom = OBAtom()
> mol.AddAtom(atom)
>
> Is one of these forms preferred?
But you just suggested a second way to add a bond. :-) But you're
right - the NewAtom, AddAtom stuff is confusing. If you use NewAtom,
you are letting yourself in for a world of trouble, because the
NewAtom then needs to be added to the molecule and I'm not sure that
that works correctly from Python (I think a copy of the OBAtom is
added instead due to some SWIG unmagic).
> 9) How does OpenBabel define "implicit hydrogen"? It's different
> than my definition, which in
>
> [NH3].C
>
> is the explicitly stated implicit h-count in [NH3] and the
> inferred H-count in C (which is 4). OpenBabel treats the H3
> as explicit.
>
> It appears that OpenBabel doesn't have an implicit h-count
> field and instead tracks the valence, then calculates the
> h-count based on the valence.
?
> 10) Shouldn't the molecular formula for "[NH4+]" include
> the total formal charge?
Sounds like a bug.
> 11) The formal results for c1ccccc1O.[NH4+] are very strange
>
> Take a look at this test
>
> self.assertEquals(mol.GetSpacedFormula(), "C 6 H 10 N 1 O 1 ")
> self.assertEquals(mol.GetSpacedFormula(1), "C 6 H 10 N O ")
>
> Why is there a double space in "N 1" and "O 1"? Why is there a
> space after the last "1"? And just how useful is the "include
> implicit hydrogens" flag to generate
>
> self.assertEquals(mol.GetSpacedFormula(0, ' ', 0), "C 6 H 4 N 1 O 1
> ")
>
> ?
Sounds like a bug.
> 12) I don't think atom.GetCoordinate() is accessible from Python
> except through ctypes (Python's foreign function interface)
I think no C arrays are accessible (I think most instances can be
worked around). You need to use atom.GetVector().
> 13) Why does GetAtom() start from 1 while GetBond() start from 0?
> (Perhaps this is in the documentation somewhere, but I was on a
> train without access to the internet and Google)
>
> mol = parse_smiles("C#N")
> C = mol.GetAtom(1)
> N = mol.GetAtom(2)
> # XXX Why do bonds starts from 0 and not 1
> self.assertEquals(mol.GetBond(1), None)
Legacy. They should both start from 0.
> 14) The documentation for OBBond.GetNbrAtom needs to warn that if
> the atom isn't one of the bond atoms then the GetBeginAtom() will
> be returned. (This documentation for GetNbrAtomIdx already
> has a warning to that effect.)
Documentation bug.
> 15) In an OBRing, if there are multiple non-C atoms, which one
> is the root? And since ring.GetRootAtom() returns an integer,
> shouldn't it be named GetRootAtomIdx()?
?
> 16) Why does GetAngle() return degrees instead of radians? I see
> the warning to that effect, but it was rather unexpected to see.
Legacy.
> 17) If you have something like "CNOS" then C.IsConnected(C) is
> true. I did not expect that. This is because IsConnected checks
> to see if the target is any of the atoms of either side of the
> bonds that are connected to C. And of course, C is always one
> of the atoms connected to a bond of C.
>
> This has a downstream consequence in that
>
> C.IsOneFour(C) == True
>
> If I read the code right, the C in "CN" will also be in a
> 1-4 relationship with itself.
Sounds like a bug. Nice catch.
> 18) There are some minor problems with the spectrophoretest.cpp
> test01. (I didn't review all of the tests.)
>
> In
>
> OB_REQUIRE( (r[13] > 3.470) && (r[13] < 3.472) );
> OB_REQUIRE( (r[14] > 6.698) && (r[14] < 6.800) );
> OB_REQUIRE( (r[15] > 9.485) && (r[15] < 9.487) );
>
> the line with r[14] should have 6.700 as the upper bound.
>
> and
> OB_REQUIRE( (r[18] > 4.899) && (r[18] < 4.902) );
> should be slightly smaller in the upper bound
> OB_REQUIRE( (r[18] > 4.899) && (r[18] < 4.901) );
>
>
> 19) I see that MMFF94's ValidateGradients() dumps output
> to stdout. The other ValidateGradients I tested (gaff and
> ghemical) do not do that, and the output probably isn't
> appropriate for a library.
>
> Then again, the API docs say that it's meant for debugging.
> Should the debugging API be published for general use?
?
> 20) OBAtom.HtoMethtyl() can also dump code to stderr
>
> v3: < -0.28, 0, 0 > v4: < -0.28, 0, 0 >
>
> This is quite clearly a bit of debugging code which
> wasn't removed:
>
> cerr << "v3: " << v3 << " v4: " << v4 << endl;
>
I guess it should be removed, so.
> Cheers!
>
>
> Andrew
> da...@dalkescientific.com
>
>
>
> ------------------------------------------------------------------------------
> Centralized Desktop Delivery: Dell and VMware Reference Architecture
> Simplifying enterprise desktop deployment and management using
> Dell EqualLogic storage and VMware View: A highly scalable, end-to-end
> client virtualization framework. Read more!
> http://p.sf.net/sfu/dell-eql-dev2dev
> _______________________________________________
> OpenBabel-discuss mailing list
> openbabel-disc...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openbabel-discuss
>
------------------------------------------------------------------------------
Centralized Desktop Delivery: Dell and VMware Reference Architecture
Simplifying enterprise desktop deployment and management using
Dell EqualLogic storage and VMware View: A highly scalable, end-to-end
client virtualization framework. Read more!
http://p.sf.net/sfu/dell-eql-dev2dev
_______________________________________________
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel
