open-reaction-database / ord-schema

Schema for the Open Reaction Database
https://open-reaction-database.org
Apache License 2.0
93 stars 26 forks source link

Push to PyPI after every merge #630

Closed skearnes closed 2 years ago

skearnes commented 2 years ago

Resolves #629

codecov[bot] commented 2 years ago

Codecov Report

Merging #630 (ef95ecd) into main (e51ca0b) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #630   +/-   ##
=======================================
  Coverage   64.50%   64.50%           
=======================================
  Files          16       16           
  Lines        1854     1854           
  Branches      497      497           
=======================================
  Hits         1196     1196           
  Misses        556      556           
  Partials      102      102           
skearnes commented 2 years ago

@connorcoley: should we create a conda package instead of a pypi package to preserve the conda rdkit installation path? I'm a bit concerned about possible conflicts between conda rdkit and rdkit-pypi if we expect most people are still following the recommended installation path; see https://github.com/rdkit/rdkit/issues/1812#issuecomment-1153095189.

skearnes commented 2 years ago

@connorcoley @ipendlet after some digging I find myself thinking that we should provide both conda and pypi packages. That said, if we have to pick one to start with, conda might be a better path since that will avoid issues like this one: https://github.com/kuelumbus/rdkit-pypi/issues/48. WDYT?

skearnes commented 2 years ago

@connorcoley @ipendlet after some digging I find myself thinking that we should provide both conda and pypi packages. That said, if we have to pick one to start with, conda might be a better path since that will avoid issues like this one: kuelumbus/rdkit-pypi#48. WDYT?

@connorcoley @ipendlet I think this question might become moot if rdkit-pypi is renamed to rdkit though.

skearnes commented 2 years ago

@connorcoley @ipendlet I thought of a workaround for the conda/pip issues prior to any renaming. I'll use the extras_require logic to only install rdkit-pypi if explicitly requested and update the readme to indicate that you should only use that if you haven't installed rdkit already via conda.