[WIP] Refactoring

[nividic]

Please check the changes and DO NOT MERGE. I coded the part that we agreed.

[ppxasjsm]

In what way do you want this reviewed? There are a lot of code changes in two commits. Some questions to make this easier for me:

• Why did you change from GPL to MIT? Is there a specific reason?
• Do you wish to merge some of the changes? How do you wish to merge them?

I'll try and install the changes with the conda env you suggested and try and give some feedback. I am just a bit unclear how to proceed with the diverging code.

[ppxasjsm]

Hi @nividic,

Just a quick heads up following your installation instructions I end up with the following error:

conda env create -f env.yaml -n lomap_refactor
Solving environment: done

==> WARNING: A newer version of conda exists. <==
  current version: 4.5.11
  latest version: 4.5.12

Please update conda by running

    $ conda update -n base -c defaults conda

Downloading and Extracting Packages
libtiff-4.0.10       | 486 KB    | #################################################################################### | 100% 
pandas-0.23.4        | 9.1 MB    | #################################################################################### | 100% 
libiconv-1.15        | 1.3 MB    | #################################################################################### | 100% 
pillow-5.4.1         | 575 KB    | #################################################################################### | 100% 
glib-2.58.2          | 3.1 MB    | #################################################################################### | 100% 
rdkit-2018.09.1.0    | 18.8 MB   | #################################################################################### | 100% 
olefile-0.46         | 31 KB     | #################################################################################### | 100% 
gettext-0.19.8.1     | 3.4 MB    | #################################################################################### | 100% 
bzip2-1.0.6          | 148 KB    | #################################################################################### | 100% 
libxml2-2.9.8        | 1.9 MB    | #################################################################################### | 100% 
fontconfig-2.13.1    | 275 KB    | #################################################################################### | 100% 
cairo-1.16.0         | 1.3 MB    | #################################################################################### | 100% 
pytz-2018.9          | 229 KB    | #################################################################################### | 100% 
pixman-0.34.0        | 597 KB    | #################################################################################### | 100% 
freetype-2.9.1       | 868 KB    | #################################################################################### | 100% 
pcre-8.41            | 222 KB    | #################################################################################### | 100% 
Preparing transaction: done
Verifying transaction: done
Executing transaction: done
Collecting Openeye-toolkits==2018.10.1 (from -r /mycomputer/Lomap/condaenv.7yg5rn17.requirements.txt (line 1))
  Could not find a version that satisfies the requirement Openeye-toolkits==2018.10.1 (from -r /mycomputer/Lomap/condaenv.7yg5rn17.requirements.txt (line 1)) (from versions: )
No matching distribution found for Openeye-toolkits==2018.10.1 (from -r /mycomputer/Lomap/condaenv.7yg5rn17.requirements.txt (line 1))

CondaValueError: pip returned an error

[ppxasjsm]

conda install -c openeye openeye-toolkits

Seems to fix the problem above.

[ppxasjsm]

So I managed to get the example to work, well to the extent that I don't have an openeye licence.

I am struggling to understand the logic a little though. Can you make create a diagram that explains your design logic a bit better? If the molecule class can contain different types of molecules, i.e. rdkit or Openeye ones, does this not mean we are essentially writing everything twice (or more times should an additional toolkit be supported), because we need to include all functionalities for all toolkits and just end up with a lot of duplication of code?

[nividic]

@ppxasjsm The problem with the error in the package installation is related to the conda channel....I'll add it to the env file soon. I'll try to make a digram asap but in the end whatever we do we have to write code for the different toolkits that performs the same task e.g. the mcs calculation is done by both toolkits but the APIs are different so you will end up writing the same functionalities by using different toolkits (it is not the same as code duplication). In the current refactoring the read_molecule function can give you an idea of it:

https://github.com/MobleyLab/Lomap/blob/refactoring/lomap/toolkits/oe.py#L28-L83 https://github.com/MobleyLab/Lomap/blob/refactoring/lomap/toolkits/rdk.py#L32-L83

but the api lomap function is in common:

https://github.com/MobleyLab/Lomap/blob/refactoring/lomap/toolkits/toolkits.py#L39-L46

If you have better solutions please let us know.

[ppxasjsm]

@nividic I think I have a bit of a better idea what you want to achieve. I am worried that you might end up with a lot of diverging code, because different toolkits support different functionality.

I think I have a bit of a better idea what we might want to do with lomap or parts of it and what kind of modularity would be good for us. I'll try and summarise this in a small write up and make an issue with it.

[davidlmobley]

Closing this, since this is no longer the plan. :)