Skip to content

Edits to User Guide documentation #995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
atravitz merged 25 commits into from
Nov 15, 2024
Merged

Edits to User Guide documentation #995

atravitz merged 25 commits into from
Nov 15, 2024

Conversation

@atravitz
Copy link
Contributor

@atravitz atravitz commented Oct 31, 2024 •
edited
Loading

Edits just to the top level and Setup section. Further edits for other sections (and to update graphics) will follow in later PRs.

Developers certificate of origin

@codecov
Copy link

codecov bot commented Oct 31, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.84%. Comparing base (4c06370) to head (6e44bfb).
Report is 203 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #995      +/-   ##
==========================================
- Coverage   94.59%   92.84%   -1.76%     
==========================================
  Files         134      134              
  Lines        9940     9940              
==========================================
- Hits         9403     9229     -174     
- Misses        537      711     +174
Flag Coverage Δ
fast-tests 92.84% <ø> (?)
slow-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@atravitz atravitz marked this pull request as ready for review November 1, 2024 21:38
@atravitz atravitz requested a review from mikemhenry November 1, 2024 21:39
IAlibay
IAlibay requested changes Nov 5, 2024
View reviewed changes
Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to put a block here because it looks like some of the things here will need some discussion / will have differing views.

atravitz and others added 2 commits November 5, 2024 14:33
@atravitz @IAlibay
Update docs/guide/setup/defining_protocols.rst
e192696 
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
@atravitz @IAlibay
Update docs/guide/setup/index.rst
e2c6c4e 
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
ijpulidos
ijpulidos reviewed Nov 8, 2024
View reviewed changes
Copy link
Contributor

@ijpulidos ijpulidos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much clearer and concise, great work! I just added some comments.

hannahbaumann
hannahbaumann requested changes Nov 11, 2024
View reviewed changes
Copy link
Contributor

@hannahbaumann hannahbaumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for you all your work on this @atravitz , this looks great overall! Just left a few suggestions!

atravitz and others added 10 commits November 12, 2024 11:07
@atravitz
adding links to introduction
0b2978d
@atravitz @ijpulidos
@hannahbaumann @IAlibay
Apply suggestions from code review
3895871 
Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>
Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
@atravitz
Merge branch 'main' into docs_user_guide_edits
8ad09d1
@atravitz @ijpulidos
Apply suggestions from code review
5a7737c 
Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>
@atravitz
fixing bash rendering
83d1093
@atravitz
fixing typo
360e317
@atravitz
making language clearer
93eb486
@atravitz
making protocol description clearer
10e13e7
@atravitz
renaming to Results Gathering
96cb6f1
@atravitz
back to bold
6dcf884
atravitz
atravitz commented Nov 13, 2024
View reviewed changes
Copy link
Contributor Author

@atravitz atravitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IAlibay A couple items I addressed but you should take one more look at. I think we need to revisit the ligand network section as a whole. Maybe a follow-up PR?

docs/guide/setup/creating_atom_mappings_and_scores.rst
The :func:`default_lomap_score` function combines several criteria
(such as the number of heavy atoms, if certain chemical changes are present,
and if ring sizes are being mutated), into a single value.
It is possible to combine scoring functions in this way because each scoring function returns a normalized value.
Copy link
Contributor Author

@atravitz atravitz Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me know if this is better.

mikemhenry
mikemhenry reviewed Nov 13, 2024
View reviewed changes
mikemhenry
mikemhenry reviewed Nov 13, 2024
View reviewed changes
@mikemhenry
Copy link
Contributor

mikemhenry commented Nov 13, 2024

+1 to removing see also from the TOC but keeping that section on the page
image

@mikemhenry
Copy link
Contributor

mikemhenry commented Nov 13, 2024

same with references
image

@mikemhenry
Copy link
Contributor

mikemhenry commented Nov 13, 2024

Also while we are at it, I think we should have a call to action on this page:
https://openfe--995.org.readthedocs.build/en/995/guide/introduction.html

In between the Introduction and Workflow overview section as a .. note:: that is something like "If you have used openfe, please use this citation xxx." I think some sentence about also sponsoring the project would be good as well.

Also also, feel free to just convert this comment into an issue if you think it warrants further discussion and/or is outside the scope of the PR.

atravitz and others added 2 commits November 14, 2024 12:53
@atravitz
making warning more visible
0c8f130
@atravitz @mikemhenry
Apply suggestions from code review
4273a36 
Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
hannahbaumann
hannahbaumann approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@hannahbaumann hannahbaumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, lgtm!

@atravitz atravitz merged commit 9b0b4b2 into main Nov 15, 2024
9 checks passed
@atravitz atravitz deleted the docs_user_guide_edits branch November 15, 2024 14:57
@atravitz atravitz mentioned this pull request Nov 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ijpulidos ijpulidos ijpulidos left review comments

@IAlibay IAlibay IAlibay requested changes

@mikemhenry mikemhenry mikemhenry approved these changes

@hannahbaumann hannahbaumann hannahbaumann approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@atravitz @mikemhenry @ijpulidos @IAlibay @hannahbaumann