Skip to content

Make OEIUPAC optional for normalize_molecule, cut release#263

Merged
Lnaden merged 7 commits into
masterfrom
iupac_fix
Feb 16, 2018
Merged

Make OEIUPAC optional for normalize_molecule, cut release#263
Lnaden merged 7 commits into
masterfrom
iupac_fix

Conversation

@Lnaden

@Lnaden Lnaden commented Feb 15, 2018

Copy link
Copy Markdown
Contributor

Also fixed typo in the error messages for OEIUPAC which incorrectly told the user it was missing OEOmega

This is actually a critical bugfix for some users who have everything needed but IUPAC and its inhibiting some packages which depend on this package.

Also fixed typo in the error messages for OEIUPAC which incorrectly told the user it was missing OEOmega

This is actually a critical bugfix for some users who have everything needed but IUPAC and its inhibiting some packages which depend on this package.
@Lnaden Lnaden requested a review from andrrizzi February 15, 2018 13:59
@Lnaden

Lnaden commented Feb 15, 2018

Copy link
Copy Markdown
Contributor Author

@jchodera do you think you can update the OpenEye License on here? I think it expired based on the logs.

@jchodera

Copy link
Copy Markdown
Member

Will do!

@Lnaden

Lnaden commented Feb 15, 2018

Copy link
Copy Markdown
Contributor Author

Thanks!

@andrrizzi andrrizzi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

import pandas as pd
import mdtraj as md
from mdtraj.testing import raises
from numpy.testing import assert_raises

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my curiosity, are there advantages of using numpy.testing.assert_raises over nose.tools.assert_raises?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nose test is the correct way to go, but this minimizes package drift, which I was trying to do with this PR. mdtraj no longer has raises, but now just passes through numpy.testing.assert_raises as its own, so I just pulled that in to avoid having people with mdtraj get an error based on version.

In some larger PR/cleanup, I fully think this should be replaced with nose's assert_raises function.

@Lnaden Lnaden merged commit 1450505 into master Feb 16, 2018
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

Successfully merging this pull request may close these issues.

3 participants