Menu ▾ ▴

#1232 exception suggested in AtomContainerSetManipulator.java

cdk-1.4.x
open
Egon Willighagen
cdk-standard module (50)
5
2012-11-03
2012-05-30
Ralf Stephan
No

AtomContainerSetManipulator.java:192

Here is an atom that isn't contained in any container which shouldn't happen --- but does with JCP in certain cases. Do you agree to throw here?

Discussion

  • Ralf Stephan

    Ralf Stephan - 2012-05-30

    This may not be a clear case. If you don't agree, it would lead to an exception in ChemModelManipulator:170, anyway

     

  • Egon Willighagen

    Egon Willighagen - 2012-05-31

    Ralf, like with the ChemModelManipulator report, I think throwing an exception is a good idea, and that we should become more strict...

    But it is a API change, and I would prefer to keep cdk-1.4.x API stable (users are kinda almost demanding that)...

    Master has not diverged much yet, and maybe I can try to convince you to rebase CDK-JChemPaint and JChemPaint on master? For CDK-JChemPaint I expect little effort... (and I need to do this anyway...)

     

  • Ralf Stephan

    Ralf Stephan - 2012-05-31

    Egon, can you please explain to me what you mean with "rebase" in
    1. connection with JCP, make sure it works with cdk/cdk master? Well it does with 1.4.10, so it should then with master, shouldn't it?
    2. in connection with cdk-jchempaint, you don't mean "git rebase", do you? why should I work on that patch, except for backporting stuff to cdk, which isn't in my focus atm? (and I'm starting to multitask between moving the JCP trac issues and Daniel's code review).