Merge pull request #42 from FelixBaensch/production

Update of ErtlFunctionalGroupsFinder v1.2 to v1.3

build.gradle

@@ -31,7 +31,7 @@ dependencies {
     implementation group: 'org.openscience.cdk', name: 'cdk-bundle', version: cdkVersion
     implementation group: 'org.openscience.cdk', name: 'cdk-scaffold', version: '2.8'
     implementation group: 'io.github.jonasschaub', name: 'sru', version: '1.4.0.0'
-    implementation group: 'io.github.jonasschaub', name: 'ErtlFunctionalGroupsFinder', version: '1.2.0.0'
+    implementation group: 'io.github.jonasschaub', name: 'ErtlFunctionalGroupsFinder', version: '1.3.0.0'
     implementation group: 'com.github.librepdf', name: 'openpdf', version: '1.3.26'
     implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'win'
     implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'linux'

src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/ErtlFunctionalGroupsFinderFragmenter.java

@@ -20,11 +20,6 @@
 
 package de.unijena.cheminf.mortar.model.fragmentation.algorithm;
 
-/**
- * TODO:
- * -
- */
-
 import de.unijena.cheminf.mortar.gui.util.GuiUtil;
 import de.unijena.cheminf.mortar.message.Message;
 import de.unijena.cheminf.mortar.model.io.Importer;
@@ -38,7 +33,6 @@
 
 import org.openscience.cdk.aromaticity.Aromaticity;
 import org.openscience.cdk.aromaticity.ElectronDonation;
-import org.openscience.cdk.exception.CDKException;
 import org.openscience.cdk.graph.ConnectivityChecker;
 import org.openscience.cdk.graph.CycleFinder;
 import org.openscience.cdk.graph.Cycles;
@@ -82,10 +76,9 @@ public static enum FGEnvOption {
         FULL_ENVIRONMENT(ErtlFunctionalGroupsFinder.Mode.NO_GENERALIZATION),
 
         /**
-         * Return only the marked atoms of a functional group, no environment. The EFGF mode for generalization is
-         * associated but the returned FG need additional processing to only return the marked atoms.
+         * Return only the marked atoms of a functional group, no environment.
          */
-        NO_ENVIRONMENT(ErtlFunctionalGroupsFinder.Mode.DEFAULT);
+        NO_ENVIRONMENT(ErtlFunctionalGroupsFinder.Mode.ONLY_MARKED_ATOMS);
 
         /**
          * The ErtlFunctionalGroupsFinder mode to use in the respective cases.
@@ -250,6 +243,11 @@ public static enum CycleFinderOption {
      */
     public static final boolean FILTER_SINGLE_ATOMS_OPTION_DEFAULT = true;
 
+    /**
+     * Default option for whether input restrictions (no metal, metalloids, pseudo atoms, charges or unconnected structures) should be applied.
+     */
+    public static final boolean APPLY_INPUT_RESTRICTIONS_OPTION_DEFAULT = false;
+
     /**
      * Cycle finder algorithm that is used should the set option cause an IntractableException.
      */
@@ -291,6 +289,7 @@ public static enum CycleFinderOption {
     //</editor-fold>
     //
     //<editor-fold desc="Private final variables">
+
     //note: since Java 21, the javadoc build complains about "double comments" when there is a comment
     // for the get method of the property and the private property itself as well
     private final SimpleEnumConstantNameProperty environmentModeSetting;
@@ -308,6 +307,8 @@ public static enum CycleFinderOption {
 
     private final SimpleBooleanProperty filterSingleAtomsSetting;
 
+    private final SimpleBooleanProperty applyInputRestrictionsSetting;
+
     /**
      * All settings of this fragmenter, encapsulated in JavaFX properties for binding in GUI.
      */
@@ -329,7 +330,7 @@ public static enum CycleFinderOption {
      * Constructor, all settings are initialised with their default values as declared in the respective public constants.
      */
     public ErtlFunctionalGroupsFinderFragmenter() {
-        int tmpNumberOfSettingsForTooltipMapSize= 6;
+        int tmpNumberOfSettingsForTooltipMapSize= 7;
         int tmpInitialCapacityForSettingNameTooltipTextMap = CollectionUtil.calculateInitialHashCollectionCapacity(
                 tmpNumberOfSettingsForTooltipMapSize,
                 BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR);
@@ -446,13 +447,18 @@ public void set(String newValue) throws NullPointerException, IllegalArgumentExc
                 ErtlFunctionalGroupsFinderFragmenter.FILTER_SINGLE_ATOMS_OPTION_DEFAULT);
         this.settingNameTooltipTextMap.put(this.filterSingleAtomsSetting.getName(),
                 Message.get("ErtlFunctionalGroupsFinderFragmenter.filterSingleAtomsSetting.tooltip"));
-        this.settings = new ArrayList<Property>(6);
+        this.applyInputRestrictionsSetting = new SimpleBooleanProperty(this, "Apply input restrictions setting",
+                ErtlFunctionalGroupsFinderFragmenter.APPLY_INPUT_RESTRICTIONS_OPTION_DEFAULT);
+        this.settingNameTooltipTextMap.put(this.applyInputRestrictionsSetting.getName(),
+                Message.get("ErtlFunctionalGroupsFinderFragmenter.applyInputRestrictionsSetting.tooltip"));
+        this.settings = new ArrayList<Property>(7);
         this.settings.add(this.fragmentSaturationSetting);
         this.settings.add(this.electronDonationModelSetting);
         this.settings.add(this.cycleFinderSetting);
         this.settings.add(this.environmentModeSetting);
         this.settings.add(this.returnedFragmentsSetting);
         this.settings.add(this.filterSingleAtomsSetting);
+        this.settings.add(this.applyInputRestrictionsSetting);
     }
     //</editor-fold>
     //
@@ -584,6 +590,24 @@ public boolean getFilterSingleAtomsSetting() {
     public SimpleBooleanProperty filterSingleAtomsSettingProperty() {
         return this.filterSingleAtomsSetting;
     }
+
+    /**
+     * Returns the boolean value of the apply strict input restrictions setting.
+     *
+     * @return true if strict input restrictions are applied to the input molecules
+     */
+    public boolean getApplyInputRestrictionsSetting() {
+        return this.applyInputRestrictionsSetting.get();
+    }
+
+    /**
+     * Returns the property object of the apply strict input restrictions setting that can be used to configure this setting.
+     *
+     * @return property object of the apply strict input restrictions setting
+     */
+    public SimpleBooleanProperty applyInputRestrictionsSettingProperty() {
+        return this.applyInputRestrictionsSetting;
+    }
     //</editor-fold>
     //
     //<editor-fold desc="Public properties set">
@@ -706,6 +730,17 @@ public void setCycleFinderSetting(CycleFinderOption anOption) throws NullPointer
     public void setFilterSingleAtomsSetting(boolean aBoolean) {
         this.filterSingleAtomsSetting.set(aBoolean);
     }
+
+    /**
+     * Sets the apply strict input restrictions setting. If true, molecules containing metal, metalloid, or pseudo atoms,
+     * formal charges, or multiple unconnected parts are filtered from the input
+     * molecules and no functional groups are determined for them.
+     *
+     * @param aBoolean true if strict input restrictions should be applied; false otherwise
+     */
+    public void setApplyInputRestrictionsSetting(boolean aBoolean) {
+        this.applyInputRestrictionsSetting.set(aBoolean);
+    }
     //</editor-fold>
     //
     //<editor-fold desc="IMoleculeFragmenter methods">
@@ -764,6 +799,7 @@ public IMoleculeFragmenter copy() {
         tmpCopy.setFragmentSaturationSetting(this.fragmentSaturationSetting.get());
         tmpCopy.setReturnedFragmentsSetting(this.returnedFragmentsSetting.get());
         tmpCopy.setFilterSingleAtomsSetting(this.filterSingleAtomsSetting.get());
+        tmpCopy.setApplyInputRestrictionsSetting(this.applyInputRestrictionsSetting.get());
         return tmpCopy;
     }
 
@@ -780,6 +816,7 @@ public void restoreDefaultSettings() {
         this.fragmentSaturationSetting.set(IMoleculeFragmenter.FRAGMENT_SATURATION_OPTION_DEFAULT.name());
         this.returnedFragmentsSetting.set(ErtlFunctionalGroupsFinderFragmenter.RETURNED_FRAGMENTS_OPTION_DEFAULT.name());
         this.filterSingleAtomsSetting.set(ErtlFunctionalGroupsFinderFragmenter.FILTER_SINGLE_ATOMS_OPTION_DEFAULT);
+        this.applyInputRestrictionsSetting.set(ErtlFunctionalGroupsFinderFragmenter.APPLY_INPUT_RESTRICTIONS_OPTION_DEFAULT);
     }
 
     @Override
@@ -793,13 +830,8 @@ public List<IAtomContainer> fragmentMolecule(IAtomContainer aMolecule)
         }
         //</editor-fold>
         IAtomContainer tmpMoleculeClone = aMolecule.clone();
-        try {
-            ErtlFunctionalGroupsFinderUtility.perceiveAtomTypesAndConfigureAtoms(tmpMoleculeClone);
-            ErtlFunctionalGroupsFinderUtility.applyAromaticityDetection(tmpMoleculeClone, this.aromaticityModelInstance);
-        } catch (CDKException anException) {
-            this.logger.log(Level.WARNING, anException.toString(), anException);
-            throw new IllegalArgumentException("Unexpected error at aromaticity detection: " + anException.toString());
-        }
+        //throws IllegalArgumentException if anything goes wrong
+        ErtlFunctionalGroupsFinder.applyPreprocessing(tmpMoleculeClone, this.aromaticityModelInstance);
         int tmpInitialCapacityForIdToAtomMap = CollectionUtil.calculateInitialHashCollectionCapacity(tmpMoleculeClone.getAtomCount(), BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR);
         HashMap<Integer, IAtom> tmpIdToAtomMap = new HashMap<>(tmpInitialCapacityForIdToAtomMap, BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR);
         for (int i = 0; i < tmpMoleculeClone.getAtomCount(); i++) {
@@ -811,13 +843,7 @@ public List<IAtomContainer> fragmentMolecule(IAtomContainer aMolecule)
         List<IAtomContainer> tmpNonFGFragments = null;
         try {
             //generate FG fragments using EFGF
-            if (this.environmentModeSetting.get().equals(FGEnvOption.NO_ENVIRONMENT.name())) {
-                //extract only marked atoms, use implemented utility method from EFGFUtilities
-                tmpFunctionalGroupFragments = ErtlFunctionalGroupsFinderUtility.findMarkedAtoms(tmpMoleculeClone);
-            } else {
-                //generalization or full environment, can both be handled by EFGF alone
-                tmpFunctionalGroupFragments = this.ertlFGFInstance.find(tmpMoleculeClone, false);
-            }
+            tmpFunctionalGroupFragments = this.ertlFGFInstance.find(tmpMoleculeClone, false, this.applyInputRestrictionsSetting.get());
             if (!tmpFunctionalGroupFragments.isEmpty()) {
                 for (IAtomContainer tmpFunctionalGroup : tmpFunctionalGroupFragments) {
                     //post-processing FG fragments
@@ -896,27 +922,27 @@ public boolean shouldBeFiltered(IAtomContainer aMolecule) {
         if (Objects.isNull(aMolecule) || aMolecule.isEmpty()) {
             return true;
         }
-        //throws NullpointerException if molecule is null
-        return ErtlFunctionalGroupsFinderUtility.shouldBeFiltered(aMolecule, this.filterSingleAtomsSetting.get());
+        if (this.filterSingleAtomsSetting.get() && ErtlFunctionalGroupsFinderUtility.isAtomOrBondCountZero(aMolecule)) {
+            return true;
+        }
+        if (this.applyInputRestrictionsSetting.get()) {
+            return !ErtlFunctionalGroupsFinder.isValidInputMoleculeWithRestrictionsTurnedOn(aMolecule);
+        }
+        return false;
     }
 
     @Override
     public boolean shouldBePreprocessed(IAtomContainer aMolecule) throws NullPointerException {
         Objects.requireNonNull(aMolecule, "Given molecule is null.");
-        //throws NullpointerException if molecule is null
-        return ErtlFunctionalGroupsFinderUtility.shouldBePreprocessed(aMolecule);
+        return false;
     }
 
     @Override
     public boolean canBeFragmented(IAtomContainer aMolecule) throws NullPointerException {
         Objects.requireNonNull(aMolecule, "Given molecule is null.");
         boolean tmpShouldBeFiltered = this.shouldBeFiltered(aMolecule);
         boolean tmpShouldBePreprocessed = this.shouldBePreprocessed(aMolecule);
-        if (tmpShouldBeFiltered || tmpShouldBePreprocessed) {
-            return false;
-        }
-        //throws NullpointerException if molecule is null
-        return ErtlFunctionalGroupsFinderUtility.isValidArgumentForFindMethod(aMolecule, this.filterSingleAtomsSetting.get());
+        return !(tmpShouldBeFiltered || tmpShouldBePreprocessed);
     }
 
     @Override
@@ -926,26 +952,30 @@ public IAtomContainer applyPreprocessing(IAtomContainer aMolecule) throws NullPo
         if (tmpShouldBeFiltered) {
             throw new IllegalArgumentException("The given molecule cannot be preprocessed but should be filtered.");
         }
+        return aMolecule.clone();
+        //Deprecated!
+        /*
         if (!this.shouldBePreprocessed(aMolecule)) {
             return aMolecule.clone();
         }
         IAtomContainer tmpPreprocessedMolecule = aMolecule.clone();
-        if (ErtlFunctionalGroupsFinderUtility.isStructureUnconnected(tmpPreprocessedMolecule)) {
+        if (ErtlFunctionalGroupsFinder.isStructureUnconnected(tmpPreprocessedMolecule)) {
             tmpPreprocessedMolecule = ErtlFunctionalGroupsFinderUtility.selectBiggestUnconnectedComponent(tmpPreprocessedMolecule);
         }
-        if (ErtlFunctionalGroupsFinderUtility.isMoleculeCharged(tmpPreprocessedMolecule)) {
+        if (ErtlFunctionalGroupsFinder.containsChargedAtom(tmpPreprocessedMolecule)) {
             try {
                 ErtlFunctionalGroupsFinderUtility.neutralizeCharges(tmpPreprocessedMolecule);
             } catch (CDKException anException) {
                 this.logger.log(Level.WARNING, anException.toString(), anException);
-                throw new IllegalArgumentException("Unexpected error at aromaticity detection: " + anException.toString());
+                throw new IllegalArgumentException("Unexpected error at charge neutralization: " + anException.toString());
             }
         }
         if (Objects.isNull(tmpPreprocessedMolecule)) {
             throw new IllegalArgumentException("The given molecule cannot be preprocessed but should be filtered.");
         } else {
             return tmpPreprocessedMolecule;
         }
+        */
     }
     //</editor-fold>
     //

src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/IMoleculeFragmenter.java

@@ -194,12 +194,12 @@ public List<IAtomContainer> fragmentMolecule(IAtomContainer aMolecule)
     public boolean shouldBeFiltered(IAtomContainer aMolecule);
 
     /**
-     * Returns true if the given molecule can be fragmented by the respective algorithm after preprocessing. Returns
-     * false if the given molecule can be directly fragmented by the algorithm without preprocessing.
-     * Does not check whether the molecule should be filtered! But throws an exception if it is null.
+     * Returns true if the given molecule can be fragmented by the respective algorithm *after preprocessing*.
+     * Does not check whether the molecule should be filtered! It is advised to check via shouldBeFiltered() whether
+     * the given molecule should be discarded anyway before calling this function.
      *
      * @param aMolecule the molecule to check
-     * @return true if the molecule needs to be preprocessed, false if it can be fragmented directly
+     * @return true if the molecule needs to be preprocessed
      * @throws NullPointerException if the molecule is null
      */
     public boolean shouldBePreprocessed(IAtomContainer aMolecule) throws NullPointerException;

src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/SugarRemovalUtilityFragmenter.java

@@ -1173,6 +1173,7 @@ public IAtomContainer applyPreprocessing(IAtomContainer aMolecule) throws NullPo
         if (!this.shouldBePreprocessed(aMolecule)) {
             return aMolecule.clone();
         }
+        //Todo I (Jonas) would like to remove any preprocessing done by the fragmenters as soon as possible, i.e. as soon as we have central preprocessing functionalities available
         if (this.sugarRUInstance.areOnlyTerminalSugarsRemoved()) {
             boolean tmpIsConnected = ConnectivityChecker.isConnected(aMolecule);
             if (!tmpIsConnected) {

src/main/resources/de/unijena/cheminf/mortar/descriptions/tools_description.xml

@@ -28,7 +28,7 @@
     </externalTool>
     <externalTool>
         <name>ErtlFunctionalGroupsFinder</name>
-        <version>1.2.0.0</version>
+        <version>1.3.0.0</version>
         <author>Sebastian Fritsch et al</author>
         <license>LGPL v2.1 (or later)</license>
     </externalTool>

src/main/resources/de/unijena/cheminf/mortar/message/Message_en_GB.properties

@@ -327,6 +327,7 @@ ErtlFunctionalGroupsFinderFragmenter.returnedFragmentsSetting.tooltip = Defines
 ErtlFunctionalGroupsFinderFragmenter.cycleFinderSetting.tooltip = Defines which CDK cycle finder algorithm should be used for aromaticity detection
 ErtlFunctionalGroupsFinderFragmenter.electronDonationModelSetting.tooltip = Defines which CDK electron donation model should be used for aromaticity detection
 ErtlFunctionalGroupsFinderFragmenter.filterSingleAtomsSetting.tooltip = Defines whether single-atom molecules should be filtered from inputs, i.e. if true, molecules consisting of only one atom are filtered from the input molecules prior to fragmentation and no functional groups are determined for them
+ErtlFunctionalGroupsFinderFragmenter.applyInputRestrictionsSetting.tooltip = Defines whether strict input restrictions should be applied; if true, this fragmenter does not accept any molecules containing metal, metalloid, or pseudo atoms, formal charges, or multiple unconnected parts; these molecules are then filtered from the input molecules prior to fragmentation and no functional groups are determined for them
 ##SugarRemovalUtilityFragmenter##
 SugarRemovalUtilityFragmenter.returnedFragmentsSetting.tooltip = Defines which fragments should be returned, sugar moieties, the aglycone, or both
 SugarRemovalUtilityFragmenter.fragmentSaturationSetting.tooltip = Defines how open valences resulting from bond breakages during fragmentation should be saturated