volkamerlab / opencadd

A Python library for structural cheminformatics
https://opencadd.readthedocs.io
MIT License
89 stars 18 forks source link

Add opencadd.databases.klifs module #38

Closed dominiquesydow closed 3 years ago

dominiquesydow commented 3 years ago

Description

Integrate klifs_utils package into opencadd in the form of the opencadd.databases.klifs module.

Todos

Questions

Status

codecov-commenter commented 3 years ago

Codecov Report

Merging #38 into master will decrease coverage by 19.41%. The diff coverage is 27.68%.

dominiquesydow commented 3 years ago

@jaimergp, whenever the test checks are done (they are in waiting mode for quite a while already), could you review this PR, please? The main aim here is to integrate the existing klifs_utils code into the opencadd package.

In the next PR, I plan to simplify the API a bit and add unit testing, thus I suggest reviewing here only the package-level part - and the API next time.

dominiquesydow commented 3 years ago

My main question is if I put the testing and environment bits at the right position within the package (see PR Questions).

dominiquesydow commented 3 years ago

@jaimergp, coming back to the still pending checks. Is it possible that the checks "test (xxx)" are outdated? https://github.community/t/expected-waiting-for-status-to-be-reported/16727

When I look into the protected branch settings, I see (only part of it shown): https://github.com/volkamerlab/opencadd/settings/branches image

The "Test on xxx" checks match with the ci.yaml configs: https://github.com/volkamerlab/opencadd/blob/master/.github/workflows/ci.yaml

Maybe we need to check those instead?

jaimergp commented 3 years ago

Yep, good catch, this was inherited from the previous superposer configs. Reviewing now.

jaimergp commented 3 years ago

All good! This was an excellent PR. Thanks for your contribution!