search

binghong-ml / retro_star

Retro*: Learning Retrosynthetic Planning with Neural Guided A* Search
MIT License
134 stars 32 forks source link

Simplify rdchiral installation #6

Open cthoyt opened 4 years ago

cthoyt commented 4 years ago

While installing the package, I didn't understand why there was a copy of the rdchiral codebase, which appears to be available from PyPI. Could this be added to the environment.yml instead of distributing it? Or, if you've made some changes, it would be a good thing to outline what they were.

I tested that this works if you point to the canonical codebase, so I'd be happy to send a PR with the corresponding update.

binghong-ml commented 4 years ago

Thanks for the suggestion! But since we are using a modified version of the rdchiral package, we cannot replace it with the pypi version unless we've done thorough testing. We will leave that as an option for the users.

SGenheden commented 4 years ago

What modifications to RDChiral have you done?

cthoyt commented 4 years ago

I think an alternate solution that would be helpful for both me and @SGenheden would be if you forked the original RDChiral repository (https://github.com/connorcoley/rdchiral) and made the updates there. Then you would be able to do the following:

  1. Make a comparison of changes
  2. Possibly give improvements back to the developer
  3. Have a bit more of a logical way of incorporating dependencies in your setup.py