Closed ojeda-e closed 3 years ago
Hi @lilyminium @orbeckst. I would appreciate some feedback on the pushed changes.
Thanks!
Hello @ojeda-e! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
The most recent pushed changes include:
ipython
to the Visualization
page to generate the plots. (Thanks @lilyminium for the suggestion, it looks amazing!)intersphinx_mapping
to link external docs.Two points to discuss:
MembraneCurvature.results.avearge_z_surface
and other attributes on the Algorithm
page. Can I just leave it like that for now? I knwo it isn't perfect, but I tried several options and couldn't make it work as it should.Edit: I am not sure what to do about the example for the case of protein with no posres. I can't make my systems public at this very moment. Maybe there is a system with such features in tests or MDAnalysisData? I'm open to suggestions case covered :) .
Thanks!
I think this PR is ready for review.
Thanks @lilyminium for all the suggestions! @orbeckst @IAlibay @fiona-naughton I would appreciate your feedback here although I think it's ready to go. :)
Thanks for taking the time to double-check the changes and approve the PR @lilyminium! I will merge after fixing the PEP8.
Changes in this PR fixes #58
Changes include:
Pending:
Question: I left the usage page as if I were going to use a different test data file for each case.
Happy to receive any other suggestions. Thanks!
@lilyminium @orbeckst @IAlibay @fiona-naughton