Any CDK "I..." class should be created via the builder, builder.newInstance(IAtom.class, "C") can be used and pass in arguments to the constructor. However the newAtomContainer(), newAtom(), newBond() are there for speed as the rest of it is done via reflection which is best avoided. Also make sure you use cdk-silent (SilentChemObjectBuilder) and NOT cdk-data (DefaultChemObjectBuilder) objects, since the later is if you need notifications/listeners. CDK v2.0 may rename these "(Standard)ChemObjectBuilder" (silent) and "NotifyChemObjectBuilder" (data) to make it more clear which one you probably need to use.
> one component group would be assigned to *[Na+].[Cl-]* and a 2nd > component group to *c1ccccc1*? So the component grouping adds a > semantical layer on top of the individual components? Yep that's correct. On Thu, 1 Sept 2022 at 13:26, Uli Fechner <u...@pending.ai> wrote: > Hi John, > > as always - thank you for your helpful reply, much appreciated. > > I'd be all for deleting/changing AtomContainerSet - I think both of us > expressed our dissatisfaction with that class in a prior convo. > > Thanks for pointing out the issue regarding the instantiation of > AtomContainer - there certainly is a bit of "new AtomContainer" in my code > that I am going to replace. As the builder also has method calls for > handing out atoms and bonds I assume that I should replace any > instantiations that use the constructor for those objects, too? > > Regarding your code that uses the Java Spliterator Interface: > > > *IAtomContainer combined = > molset.getBuilder().newAtomContainer();molset.atomContainers().spliterator().forEachRemaining(combined::add);* > > I love the Stream API, but haven't used the Spliterator interface. My > understanding is that the spliterator (also) is about being able to > parallelize tasks which would then make the modification of collections > that sit outside the spliterator chain of calls - as is the case for the > IAtomContainer *combined* - potentially unsafe in a multithreading > environment. However, I am not sure if I understood this correctly. > > Indeed, I deal with reactions :) I understand what a ReactionRole is. But > I am not 100% sure if I understand component grouping. In your example of > reactants > > *[Na+].[Cl-].c1ccccc1>>* > > one component group would be assigned to *[Na+].[Cl-]* and a 2nd > component group to *c1ccccc1*? So the component grouping adds a > semantical layer on top of the individual components? > > Best wishes > Uli > > On Thu, Sep 1, 2022 at 5:04 PM John Mayfield <john.wilkinson...@gmail.com> > wrote: > >> Hi Uli, >> >> The code you have is the correct (almost) way to do it... I'm not sure >> a utility method is needed since it's simple/easy enough and clear what is >> happening. >> >> The more correct code should not create AtomContainer with new since the >> main class ATM is actually AtomContainer2. So things will possibly break if >> you do it with "new AtomContainer" - we can't hide the constructor as it >> will break down stream code. If we're going to break APIs I would rather >> delete AtomContainerSet :-) (it is not even a set!). >> >> >> >> >> >> *IAtomContainer mergedAtomContainer = >> atomContainerSet.getBuilder().newAtomContainer();for (IAtomContainer >> atomContainer: atomContainerSet.atomContainers()) { >> mergedAtomContainer.add(atomContainer);}* >> >> Since you can do it in two (one if you don't count the new container >> construction) >> >> >> >> *IAtomContainer combined = >> molset.getBuilder().newAtomContainer();molset.atomContainers().spliterator().forEachRemaining(combined::add);* >> >> I presume you're dealing with reactions in which case please look at the >> *ReactionManipulator.toMolecule()* function. This collapses the reaction >> into a single flat molecule, but sets the ReactionRole and ReactionGroup >> properties so you can reverse it or pick things/subset easier. In your >> function hear if you come from a reaction you will remove any component >> grouping - e.g. *[Na+].[Cl-].c1ccccc1>>* would be 2 AtomContainers in an >> AtomContainerSet, if you convert this with your method then split again you >> get 3 AtomContainers. >> >> *More info on AtomContainer2* >> >> AtomContainer2 will become AtomContainer shortly (example of errors you >> get - https://github.com/cdk/cdk/issues/607, explanation - >> https://github.com/cdk/cdk/wiki/AtomContainer2). Basically it's a >> backwards compatible way of making the containers performant, but we needed >> a staggered introduction. >> >> Best, >> John >> >> On Thu, 1 Sept 2022 at 06:08, Uli Fechner <u...@pending.ai> wrote: >> >>> Hi, >>> >>> I require the functionality to merge all IAtomContainers in an >>> AtomContainerSet into a single IAtomContainer then having several >>> disconnected components / graphs and couldn't find a suitable method in >>> AtomContainerManipulator and AtomContainerSetManipulator. >>> >>> IAtomContainer mergedAtomContainer = new AtomContainer(); >>> for (IAtomContainer atomContainer: atomContainerSet.atomContainers()) { >>> mergedAtomContainer.add(atomContainer); >>> } >>> >>> If this isn't in CDK (and it is not just me not being able to find it) I >>> am happy to make a PR by adding a method, probably to >>> AtomContainerSetManipulator. >>> >>> Best >>> Uli >>> _______________________________________________ >>> Cdk-user mailing list >>> Cdk-user@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/cdk-user >>> >>
_______________________________________________ Cdk-user mailing list Cdk-user@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/cdk-user