Add MDANCE to MDAnalysis

lexin-chen commented 3 months ago

Hi MDAnalysis team!

Hope this message finds you well. MDANCE is a flexible n-ary clustering algorithm for Molecular Dynamics trajectories. We currently have a fully deterministic k-means clustering (NANI) included a pipeline for determining the number of clusters in the system, ways to identify better cluster representatives, etc. We will be adding more clustering algorithms to this package after publishing them. In conversations with @orbeckst , he suggested that we should try to incorporate this project as an MDAkit, so we are looking forward to collaborating with you.

Best, Lexin

orbeckst commented 3 months ago

Hi @lexin-chen , great to see your MDANCE package here! I'll start reviewing and may ask others from the team for help.

orbeckst commented 3 months ago

I am following the MDAKits reviewer guide

Checklist

PR

[x] Are the automatic checks all passing?
[x] Did the docs build correctly?
[x] Is the metadata file named correctly and in the right location?

MDAKit home

[x] link ok?
[x] summary?
[x] docs?
[x] installation instructions?
[x] license?

Metadata file

Required entries

[x] project_name
[x] authors
[x] maintainers
[x] description
[x] keywords
[x] licence (really, we are using British spelling?!)
[x] project_home
[x] documentation_home
[x] documentation_type

Optional entries but required in practice

[x] install, src_install, import_name, python_requires: all automatically checked, ok when CI is green
[x] run_tests: automatic but sanity check
[x] mdanalysis_requires: manually double check

Optional entries

[x] development_status
[x] publications
[ ] changelog

orbeckst commented 3 months ago

Just to add to my review: We are not looking for complete tests of everything. This is hard to do. Instead we'd like to see a few tests that run some of your typical functions. This is typically something that you are already doing in the examples. You then add an appprpriate assert statement at the end of the test function to assert that the test output is the same as a correct answer that you stored in the tests.

orbeckst commented 3 months ago

Also, even though I included gridDataFormats as a real-life example for testing, you might be better off just looking at https://github.com/MDAnalysis/mdgeomkit/tree/main/mdgeom/tests, which uses no outdated constructs... when I started my review I only thought about mdgeomkit half-way through writing...

lexin-chen commented 3 months ago

Thanks @orbeckst for the comprehensive comments! I'll definitely look into the tests and add support for this soon!

orbeckst commented 2 months ago

Hi @lexin-chen , do you need help with anything? It would be great to have the package registered before the UGM!

lexin-chen commented 2 months ago

Thanks for checking up with me. I have written some tests on the tools and inputs folder. Once I work on the tests for nani I will submit it prob by the beginning of next week!

EDIT: okay I have created tests for the mdance/tools, mdance/inputs, and mdance/cluster/nani and the db analysis as per your suggestions! Thanks again!

lexin-chen commented 2 months ago

Hi @orbeckst , thanks for the great suggestions! as per your advice, I did create tags and release and now I have dynamic versioning as part of the package. Is there a way to recheck for the GitHub actions since I have no changes to the metadata file?

orbeckst commented 2 months ago

Excellent!

I just restarted the failed job — I can do this through the GH actions interface.

orbeckst commented 2 months ago

MDANCE is officially registered!

lexin-chen commented 2 months ago

Thanks so much for your help and tips @orbeckst! I learned a lot from this experience too. Very glad it worked out

MDAnalysis / MDAKits