choderalab / fah-xchem

Tools and infrastructure for automated compound discovery using Folding@home

MIT License

6 stars 3 forks source link

Refactored website generation under WebsiteArtifactory #151

Closed dotsdl closed 2 years ago

dotsdl commented 3 years ago

Addresses #141, #149.

Reduced the complexity of the giant generate_website function by replacing it with the WebsiteArtifactory class. Separated individual tabs of site into their own calls for scope encapsulation. Aim is to make it easier to chase down subtle issues, such as why we don't have pagination arrows for the retrospective tab.

Also, separating configuration from the generation call with attributes on the object reduces the number of args that must be passed through to called methods, and avoids the workaround used previously of functions defined within the function to take advantage of its scope.

Todos

Notable points that this PR has either accomplished or will accomplish.

[x] unit tests for the WebsiteArtifactory

Status

[x] Ready to go

codecov-commenter commented 3 years ago

Codecov Report

Merging #151 (f180aa1) into master (02b5bf5) will increase coverage by 23.34%. The diff coverage is 87.28%.

dotsdl commented 2 years ago

I have finished adding unit tests for the individual tabs of the generated website, along with the top compounds detail pages. In doing this, identified and fixed the following issues:

inconsistent document structure in top-level index.html template
defined Racemic filter in exactly one place in the library
fixed odd pagination in "Reliable Transformations" tab
moved TimeStampedAnalysis model to schemas
added getter for test data, along with minimal sprint test dataset

@jchodera this is ready for review!

dotsdl commented 2 years ago

One shortcoming yet: the minimal dataset we have included here doesn't have any retrospective transformations generated. This should be addressed in a future PR.

dotsdl commented 2 years ago

Ah, good catch on GHA! The CI action was set to look only in fah_xchem/tests, but should look in fah_xchem. I've made this change and will merge when the CI finishes: https://github.com/choderalab/fah-xchem/actions/runs/1211037745