#344 Test IChemObject set implementation independence

Needs_Review
closed
nobody
cdk-1.2.x (46)
5
2012-10-28
2011-05-22
No

Descriptors should not give different results for different IChemObject implementations (data, nonotify, ...), e.g. caused by incorrect casting.

These patches provide some code clean up (0001, 0002) and the new tests (0003, 0004). Mind you, it does not catch all problems, like casting.

Patch 0003 tests of NNMolecule and Molecule give the same result.

Patch 0004 tests of descriptors can handle AtomContainer, as the API defines.

Discussion

  • Egon Willighagen

    Mind you, patch 0004 uncovers two bugs similar to bug:

    3305581 BCUTDescriptor incorrectly assumes a particular impl

    Filing these now.

     
  • Egon Willighagen

    Patches are written against the commit with tag cdk-1.2.9.

     
  • Rajarshi Guha

    Rajarshi Guha - 2011-05-22

    I think I applied these patches to the 1.29 tag and pushed, but I don't see the patches in the log. Can you check to see if the patches made it to github? If not, how do I apply a patch to a tag?

     
  • Rajarshi Guha

    Rajarshi Guha - 2011-05-22

    I think I applied these patches to the 1.29 tag and pushed, but I don't see the patches in the log. Can you check to see if the patches made it to github? If not, how do I apply a patch to a tag?

     
  • Egon Willighagen

    I don't see them either:

    https://github.com/cdk/cdk/commits/cdk-1.2.x

    But, I added that comment just to indicate the parent patch.. they should apply to the normal cdk-1.2.x branch, I guess.

    Otherwise, given a certain hash or tag, this is a good approach:

    git checkout -b 1-testApplyPatch [tag or hash]
    git am -3 --ignore-whitespace 0001-foo.patch

    If all fine...

    git rebase cdk-1.2.x

    if that is fine too, then it applies not just to the tag (cdk-1.2.9 in this case), but also to the branch. Then:

    git checkout cdk-1.2.x
    git merge 1-testApplyPatch

    (but I think your patch made it by now ... :)

     
  • Egon Willighagen

    These have now been applied to the repository.

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks