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

rdkit-pypi dependency #10

Closed
kjelljorner opened this issue May 24, 2022 · 2 comments · Fixed by #9
Closed

rdkit-pypi dependency #10

kjelljorner opened this issue May 24, 2022 · 2 comments · Fixed by #9

Comments

@kjelljorner
Copy link

asciiMol is a really cool project, but the dependency on rdkit-pypi is quite limiting imho. There are other and arguably better ways to install RDKit, including the conda package manager. In the spirit of a terminal program, I think it would make more sense to have xyz generation from smiles as a separate first step, for example

gen_xyz_from_smiles "CCCC" > butane.xyz
asciimol butane.xyz

Anyway, if you want to keep it, I think the best thing is to make rdkit-pypi an optional dependency. It is anyway imported only within the function read_smiles.

@dewberryants
Copy link
Owner

I agree with this... I've been thinking of adding several optional dependencies that can resolve formats other than .xyz and then simply delegate in order of their availability. I'll link my ASE PR for now, and hopefully I'll get around to this soon.

@dewberryants dewberryants linked a pull request May 24, 2022 that will close this issue
@kjelljorner
Copy link
Author

I made a fork with a setup.py, mostly for installing for myself. It could be installed with pip install asciimol[rdkit] to install the optional dependency: https://github.com/kjelljorner/asciiMol/blob/05a1d8ca04e549ee6bbb9dd318a885960fcb0df8/setup.py#L20. Besides ASE, you can also take a look at cclib and chemfiles

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 a pull request may close this issue.

2 participants