Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MolVS 0.1.0 Standardization fails on Python 3 #22

Closed
thesketh opened this issue Feb 26, 2018 · 6 comments
Closed

MolVS 0.1.0 Standardization fails on Python 3 #22

thesketh opened this issue Feb 26, 2018 · 6 comments

Comments

@thesketh
Copy link

thesketh commented Feb 26, 2018

Hi Matt, a bug report for something I've noticed in the latest release.

The standardize_smiles, Standardizer().tautomer_parent and Standardizer().standardize functions all fail because of line 133. These all work okay in 0.0.9

>>> mol = Chem.MolFromSmiles('[Na]OC(=O)c1ccc(C[S+2]([O-])([O-]))cc1')
>>> Standardizer().standardize(mol)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/standardize.py", line 98, in standardize
    mol = self.normalize(mol)
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 105, in __call__
    return self.normalize(mol)
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 124, in normalize
    fragments.append(self._normalize_fragment(fragment))
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 133, in _normalize_fragment
    for n in six.moves.range(self.max_restarts):
NameError: name 'six' is not defined

I had a look at the file and it seems that the 'import six' line is missing. When this is added, I get another error, but I think this may be a Py2 vs Py3 difference:

>>> Standardizer().standardize(mol)
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/standardize.py", line 98, in standardize
   mol = self.normalize(mol)
 File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 106, in __call__
   return self.normalize(mol)
 File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 125, in normalize
   fragments.append(self._normalize_fragment(fragment))
 File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 137, in _normalize_fragment
   product = self._apply_transform(mol, normalization.transform)
 File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/utils.py", line 28, in fget_memoized
   setattr(self, attr_name, fget(self))
 File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 42, in transform
   return AllChem.ReactionFromSmarts(self.transform_str.encode('utf8'))
Boost.Python.ArgumentError: Python argument types in
   rdkit.Chem.rdChemReactions.ReactionFromSmarts(bytes)
did not match C++ signature:
   ReactionFromSmarts(char const* SMARTS, boost::python::dict replacements={}, bool useSmiles=False)

When I move ".encode('utf8')" from line 42 it works as expected in Py3, but I think this would probably produce an error on Py2.

Hope this helps, thanks for working on this, it's been really useful!

EDIT: I see that these changes are already reflected in the file on GitHub, it seems that the PyPI version is just a bit out of date

@mcs07
Copy link
Owner

mcs07 commented Feb 26, 2018

Weird, the import six line definitely should already be there:
https://github.com/mcs07/MolVS/blob/v0.1.0/molvs/normalize.py#L19

Likewise I think the string encoding for py2/py3 was fixed while ago:
0e96acf#diff-36072b6ed94d3d5dd8829d1effae2cd5

How did you install MolVS? Is it possible you have an older version? What do you see if you do:

>>> import molvs
>>> molvs.__version__
'0.1.0'

Might be worth trying to uninstall and install again.

@thesketh
Copy link
Author

Yeah, I was confused too, so I tried the reinstall a few times (pip install molvs==0.1.0 and pip install molvs==0.0.9). I'll try clearing the cached version and see if it makes a difference

[  2:23pm ]  [ travis@mljet:~ ]
 $ pip install molvs==0.1.0
Collecting molvs==0.1.0
  Using cached MolVS-0.1.0-py3-none-any.whl
Requirement already satisfied: six in ./anaconda3/envs/rdkit/lib/python3.6/site-packages (from molvs==0.1.0)
Installing collected packages: molvs
  Found existing installation: MolVS 0.0.9
    Uninstalling MolVS-0.0.9:
      Successfully uninstalled MolVS-0.0.9
Successfully installed molvs-0.1.0
[  2:23pm ]  [ travis@mljet:~ ]
 $ python
Python 3.6.4 |Anaconda, Inc.| (default, Jan 16 2018, 18:10:19)
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import molvs
>>> molvs.__version__
'0.1.0'
>>> exit()
[  2:23pm ]  [ travis@mljet:~ ]
 $ cat /home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py | grep "import six"
[  2:23pm ]  [ travis@mljet:~ ]
 $

@thesketh
Copy link
Author

Yeah, even with a forced reinstall I'm getting the same issue, it must just be the PyPI package

[  2:23pm ]  [ travis@mljet:~ ]
 $ pip install -I --no-cache-dir molvs==0.1.0
Collecting molvs==0.1.0
  Downloading MolVS-0.1.0-py3-none-any.whl
Collecting six (from molvs==0.1.0)
  Downloading six-1.11.0-py2.py3-none-any.whl
Installing collected packages: six, molvs
Successfully installed molvs-0.1.0 six-1.11.0
[  2:26pm ]  [ travis@mljet:~ ]
 $ python
Python 3.6.4 |Anaconda, Inc.| (default, Jan 16 2018, 18:10:19)
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import molvs
>>> molvs.__version__
'0.1.0'
>>> molvs.standardize_smiles('c1ccccc1')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/standardize.py", line 305, in standardize_smiles
    mol = Standardizer().standardize(mol)
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/standardize.py", line 98, in standardize
    mol = self.normalize(mol)
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 105, in __call__
    return self.normalize(mol)
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 124, in normalize
    fragments.append(self._normalize_fragment(fragment))
  File "/home/travis/anaconda3/envs/rdkit/lib/python3.6/site-packages/molvs/normalize.py", line 133, in _normalize_fragment
    for n in six.moves.range(self.max_restarts):
NameError: name 'six' is not defined

@mcs07
Copy link
Owner

mcs07 commented Feb 26, 2018

Oh dear, yes there's definitely something wrong specifically with the wheel file on PyPI. I've removed it, so pip should use the source distribution instead, which looks fine to me.

@thesketh
Copy link
Author

That's great, thanks for your quick response! Everything seems to be working perfectly now, I'll close this issue

@JoshuaMeyers
Copy link
Contributor

Great, I have just had the same issue and this fixed it. Cheers guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants