Integration of CDK ExhaustiveFragmenter #133
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,283 @@ | ||
| /* | ||
| * MORTAR - MOlecule fRagmenTAtion fRamework | ||
| * Copyright (C) 2024 Felix Baensch, Jonas Schaub ([email protected], [email protected]) | ||
| * | ||
| * Source code is available at <https://github.com/FelixBaensch/MORTAR> | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| * of this software and associated documentation files (the "Software"), to deal | ||
| * in the Software without restriction, including without limitation the rights | ||
| * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| * copies of the Software, and to permit persons to whom the Software is | ||
| * furnished to do so, subject to the following conditions: | ||
| * | ||
| * The above copyright notice and this permission notice shall be included in all | ||
| * copies or substantial portions of the Software. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
| * SOFTWARE. | ||
| */ | ||
|
|
||
| package de.unijena.cheminf.mortar.model.fragmentation.algorithm; | ||
|
|
||
| import de.unijena.cheminf.mortar.gui.util.GuiUtil; | ||
| import de.unijena.cheminf.mortar.message.Message; | ||
| import de.unijena.cheminf.mortar.model.io.Importer; | ||
| import de.unijena.cheminf.mortar.model.util.BasicDefinitions; | ||
| import de.unijena.cheminf.mortar.model.util.SimpleIDisplayEnumConstantProperty; | ||
|
|
||
| import javafx.beans.property.Property; | ||
| import javafx.beans.property.SimpleIntegerProperty; | ||
|
|
||
| import org.openscience.cdk.DefaultChemObjectBuilder; | ||
| import org.openscience.cdk.fragment.ExhaustiveFragmenter; | ||
| import org.openscience.cdk.interfaces.IAtomContainer; | ||
| import org.openscience.cdk.smiles.SmilesParser; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
|
|
||
| /** | ||
| * Wrapper class that makes the | ||
| * <a href="https://cdk.github.io/cdk/latest/docs/api/org/openscience/cdk/fragment/ExhaustiveFragmenter.html"> | ||
| * exhaustive fragmentation | ||
| * </a> | ||
| * from the CDK, available for MORTAR. | ||
| * | ||
| * @author Tom Weiß | ||
| * @version 1.0.0.0 | ||
| */ | ||
| public class CDKExhaustiveFragmenter implements IMoleculeFragmenter { | ||
| //<editor-fold desc="Private static final variables"> | ||
| /** | ||
| * The default value for the minimum fragment size used for the fragmentation. | ||
| */ | ||
| private static final int DEFAULT_MINIMUM_FRAGMENT_SIZE = 6; | ||
| // | ||
| /** | ||
| * The name of the algorithm used for fragmentation. | ||
| */ | ||
| private static final String ALGORITHM_NAME = "Exhaustive Fragmenter"; | ||
|
There was a problem hiding this comment. These two should be public. |
||
| //</editor-fold> | ||
| // | ||
| //<editor-fold desc="Private final variables"> | ||
| /** | ||
| * The minimum size of the returned fragments. This size consists of all atoms, that are connected by more than | ||
| * a single bond or have more than one single bond. | ||
| */ | ||
| private final SimpleIntegerProperty minimumFragmentSize; | ||
| // | ||
| /** | ||
| * All settings of this fragmenter, encapsulated in JavaFX properties for binding in GUI. | ||
| */ | ||
| private final List<Property<?>> settings; | ||
| // | ||
| /** | ||
| * Map to store pairs of {@literal <setting name, tooltip text>}. | ||
| */ | ||
| private final HashMap<String, String> settingNameTooltipTextMap; | ||
| // | ||
| /** | ||
| * Map to store pairs of {@literal <setting name, display name>}. | ||
| */ | ||
| private final HashMap<String, String> settingNameDisplayNameMap; | ||
| // | ||
| /** | ||
| * Instance of ExhaustiveFragmenter class to fragment a molecule. | ||
| */ | ||
| private final ExhaustiveFragmenter cdkEFInstance; | ||
| // | ||
| /** | ||
| * Logger of this class. | ||
| */ | ||
| private static final Logger LOGGER = Logger.getLogger(CDKExhaustiveFragmenter.class.getName()); | ||
| //</editor-fold> | ||
| // | ||
| //<editor-fold desc="Constructor"> | ||
| /** | ||
| * Constructor, all settings are initialised with their default values as declared in the respective public constants. | ||
| */ | ||
| public CDKExhaustiveFragmenter() { | ||
| int tmpNumberOfSettings = 2; | ||
| this.settingNameTooltipTextMap = new HashMap<>(tmpNumberOfSettings, | ||
| BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); | ||
| this.settingNameDisplayNameMap = new HashMap<>(tmpNumberOfSettings, | ||
| BasicDefinitions.DEFAULT_HASH_COLLECTION_LOAD_FACTOR); | ||
|
There was a problem hiding this comment. Please use CollectionUtil.calculateInitialHashCollectionCapacity() or HashMap.newHashMap(). There was a problem hiding this comment. I see now that you only have one setting but are initialising the maps with a capacity of 2, ok. But for the future, please use the ways I indicated. |
||
| this.cdkEFInstance = new ExhaustiveFragmenter(); | ||
| this.minimumFragmentSize = new SimpleIntegerProperty(this, | ||
|
There was a problem hiding this comment. I would put -Setting at the end of this variable or -Preference. Otherwise, one might expect an integer here. |
||
| "Minimum Size for the returned fragments", | ||
| DEFAULT_MINIMUM_FRAGMENT_SIZE) { | ||
|
There was a problem hiding this comment. Just as instance variables are indicated by the this statement, static variables are indicated by putting the class name upfront. So "CDKExhaustiveFragementer.DEFAULT_MINIMUM_FRAGMENT_SIZE". |
||
| @Override | ||
| public void set(int newValue) { | ||
| if (newValue > 0) { | ||
| try { | ||
| //throws IllegalArgumentException | ||
| CDKExhaustiveFragmenter.this.cdkEFInstance.setMinimumFragmentSize(newValue); | ||
| } catch (IllegalArgumentException anException) { | ||
|
There was a problem hiding this comment. I don't think the ExhaustiveFragmenter method throws that exception, so you don't need try-catch here. |
||
| CDKExhaustiveFragmenter.LOGGER.log(Level.WARNING, anException.toString(), anException); | ||
| GuiUtil.guiExceptionAlert(Message.get("Fragmenter.IllegalSettingValue.Title"), | ||
| Message.get("Fragmenter.IllegalSettingValue.Header"), | ||
| anException.toString(), | ||
| anException); | ||
| //re-throws the exception to properly reset the binding | ||
| throw anException; | ||
| } | ||
| super.set(newValue); | ||
| } | ||
| else { | ||
| IllegalArgumentException anException = new IllegalArgumentException("The minimum fragment size can not be zero"); | ||
| CDKExhaustiveFragmenter.LOGGER.log(Level.WARNING, anException.toString(), anException); | ||
| GuiUtil.guiExceptionAlert(Message.get("Fragmenter.IllegalSettingValue.Title"), | ||
| Message.get("Fragmenter.IllegalSettingValue.Header"), | ||
| anException.toString(), | ||
| anException); | ||
| //re-throws the exception to properly reset the binding | ||
| throw anException; | ||
| } | ||
| } | ||
| }; | ||
| this.settingNameTooltipTextMap.put(this.minimumFragmentSize.getName(), | ||
| Message.get("CDKExhaustiveFragmenter.minFragmentSize.tooltip")); | ||
| this.settingNameDisplayNameMap.put(this.minimumFragmentSize.getName(), | ||
| Message.get("CDKExhaustiveFragmenter.minFragmentSize.displayName")); | ||
| this.settings = new ArrayList<>(tmpNumberOfSettings); | ||
| this.settings.add(this.minimumFragmentSize); | ||
| } | ||
| //</editor-fold> | ||
| // | ||
| //<editor-fold desc="Public properties get"> | ||
| /** | ||
| * Returns the minimum fragment size currently set. | ||
| * | ||
| * @return the currently set minimum fragment size. | ||
| */ | ||
| public int getMinimumFragmentSize() { | ||
| return this.minimumFragmentSize.get(); | ||
|
There was a problem hiding this comment. Please add a getter for the property also. There was a problem hiding this comment. And a setter is missing. |
||
| } | ||
| //</editor-fold> | ||
| // | ||
| //<editor-fold desc="IMoleculeFragmenter methods"> | ||
| @Override | ||
|
There was a problem hiding this comment. Please add an empty line, the code fold is not working properly, otherwise (because of the @OverRide below it) |
||
| public List<Property<?>> settingsProperties() { | ||
| return this.settings; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, String> getSettingNameToTooltipTextMap() { | ||
| return this.settingNameTooltipTextMap; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, String> getSettingNameToDisplayNameMap() { | ||
| return this.settingNameDisplayNameMap; | ||
| } | ||
|
|
||
| @Override | ||
| public String getFragmentationAlgorithmName() { | ||
| return CDKExhaustiveFragmenter.ALGORITHM_NAME; | ||
| } | ||
|
|
||
| @Override | ||
| public String getFragmentationAlgorithmDisplayName() { | ||
| return Message.get("CDKExhaustiveFragmenter.displayName"); | ||
| } | ||
|
|
||
| @Override | ||
| public FragmentSaturationOption getFragmentSaturationSetting() { | ||
| //TODO: there is currently no possibility to implement saturation settings for the exhaustive fragmenter. | ||
|
There was a problem hiding this comment. Can you describe this in more detail here like you did below? |
||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public SimpleIDisplayEnumConstantProperty fragmentSaturationSettingProperty() { | ||
| //TODO: there is currently no possibility to implement saturation settings for the exhaustive fragmenter. | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public void setFragmentSaturationSetting(FragmentSaturationOption anOption) throws NullPointerException { | ||
| //TODO: there is currently no possibility to implement saturation settings for the exhaustive fragmenter. | ||
| } | ||
|
|
||
| @Override | ||
| public IMoleculeFragmenter copy() { | ||
| CDKExhaustiveFragmenter tmpCopy = new CDKExhaustiveFragmenter(); | ||
| tmpCopy.minimumFragmentSize.set(this.minimumFragmentSize.get()); | ||
| return tmpCopy; | ||
| } | ||
|
|
||
| @Override | ||
| public void restoreDefaultSettings() { | ||
| this.minimumFragmentSize.set(CDKExhaustiveFragmenter.DEFAULT_MINIMUM_FRAGMENT_SIZE); | ||
| } | ||
|
|
||
| @Override | ||
| public List<IAtomContainer> fragmentMolecule(IAtomContainer aMolecule) throws NullPointerException, IllegalArgumentException, CloneNotSupportedException { | ||
| //<editor-fold desc="Parameter tests"> | ||
| Objects.requireNonNull(aMolecule, "Given molecule is null."); | ||
| boolean tmpCanBeFragmented = this.canBeFragmented(aMolecule); | ||
| if (!tmpCanBeFragmented) { | ||
| throw new IllegalArgumentException("Given molecule cannot be fragmented but should be filtered or preprocessed first."); | ||
| } | ||
| //</editor-fold> | ||
| IAtomContainer tmpMoleculeClone = aMolecule.clone(); | ||
| List<IAtomContainer> tmpFragments = new ArrayList<>(0); | ||
|
There was a problem hiding this comment. Think of a more suiting default initial size, like maybe the number of bonds in the molecule times two? |
||
| try { | ||
| SmilesParser tmpSmilesParser = new SmilesParser(DefaultChemObjectBuilder.getInstance()); | ||
|
There was a problem hiding this comment. Please use SilentChemObjectBuilder |
||
| this.cdkEFInstance.generateFragments(tmpMoleculeClone); | ||
| // there is also an option to extract atom containers directly with getFragmentsAsContainers but this oversaturates | ||
| // fragments described in this issue https://github.com/cdk/cdk/issues/1119. | ||
|
There was a problem hiding this comment. Good that you put this here but please flag it with TODO: |
||
| List<String> tmpSmiles = new ArrayList<>(List.of(this.cdkEFInstance.getFragments())); | ||
| for (String smile : tmpSmiles) { | ||
| tmpFragments.add(tmpSmilesParser.parseSmiles(smile)); | ||
| } | ||
|
|
||
| } catch (Exception anException) { | ||
| throw new IllegalArgumentException("An error occurred during fragmentation: " + anException.toString() + " Molecule Name: " + aMolecule.getProperty(Importer.MOLECULE_NAME_PROPERTY_KEY)); | ||
| } | ||
| return tmpFragments; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean shouldBeFiltered(IAtomContainer aMolecule) { | ||
| return (Objects.isNull(aMolecule) || aMolecule.isEmpty()); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean shouldBePreprocessed(IAtomContainer aMolecule) throws NullPointerException { | ||
| Objects.requireNonNull(aMolecule, "Given molecule is null."); | ||
| 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); | ||
| return !(tmpShouldBeFiltered || tmpShouldBePreprocessed); | ||
| } | ||
|
|
||
| @Override | ||
| public IAtomContainer applyPreprocessing(IAtomContainer aMolecule) throws NullPointerException, IllegalArgumentException, CloneNotSupportedException { | ||
| Objects.requireNonNull(aMolecule, "Given molecule is null."); | ||
| boolean tmpShouldBeFiltered = this.shouldBeFiltered(aMolecule); | ||
| if (tmpShouldBeFiltered) { | ||
| throw new IllegalArgumentException("The given molecule cannot be preprocessed but should be filtered."); | ||
| } | ||
| if (!this.shouldBePreprocessed(aMolecule)) { | ||
| return aMolecule.clone(); | ||
| } | ||
| return aMolecule.clone(); | ||
|
There was a problem hiding this comment. The if statement is unnecessary. I guess you wanted to leave room between the if statement and the return statement for actual preprocessing should you need to add it later? If yes, please add a comment, at least. Or simply remove the if. |
||
| } | ||
| //</editor-fold> | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary comma