search

VlachosGroup / AIMSim

A Python toolbox to work with molecular similarity
https://vlachosgroup.github.io/AIMSim/README.html
MIT License
33 stars 4 forks source link

Loosen Dependency Requirements for Python Compatibility by Dropping Mordred #241

Closed JacksonBurns closed 1 year ago

JacksonBurns commented 1 year ago

Attempting to resolve #240 by allowing wider range of numpy versions since the latest builds have better support.

JacksonBurns commented 1 year ago

Related, #219 attempted to do this but we dropped it because it caused backwards compatibility issues with Python. Will attempt to resolve here.

JacksonBurns commented 1 year ago

mordred requires an older version of networkx to function properly with Python 3.9.

JacksonBurns commented 1 year ago

The fixed version of networkx is incompatible with moving the version of numpy up -- there is a PR open to fix this on the repo but it's stale. Looks like mordred has basically been abandoned at this point.

I think that, in order to support future Python versions as well as M1 Macs, we would need to remove mordred from the default install. We can make it an optional install, i.e. pip install aimsim[mordred] but clarify that such an install is only supported on Python 3.8 The option that I would prefer would be to fork mordred, fix the dependency issue, and then include the source code in AIMSim with the appropriate attribution. Option 3 is, of course, to just drop mordred altogether. Thoughts @himaghna ?

himaghna commented 1 year ago

The fixed version of networkx is incompatible with moving the version of numpy up -- there is a PR open to fix this on the repo but it's stale. Looks like mordred has basically been abandoned at this point.

I think that, in order to support future Python versions as well as M1 Macs, we would need to remove mordred from the default install. We can make it an optional install, i.e. pip install aimsim[mordred] but clarify that such an install is only supported on Python 3.8 The option that I would prefer would be to fork mordred, fix the dependency issue, and then include the source code in AIMSim with the appropriate attribution. Option 3 is, of course, to just drop mordred altogether. Thoughts @himaghna ?

I think for the short term we might include modred as an optional install with tags (such as -md or some such). Long term I agree moving it into our base code makes the most sense.

himaghna commented 1 year ago

Tests failing

JacksonBurns commented 1 year ago

@JacksonBurns this should be accompanied by a beta release, likely 2.0.0b0 so that other changes can continue to be made.

JacksonBurns commented 1 year ago

Also see #247 when reevaluating Python minor version compatibility and dependency version requirements.