-
Notifications
You must be signed in to change notification settings - Fork 21
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.
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
clear up language a bit for dg report help text #549
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #549 +/- ##
=======================================
Coverage 91.38% 91.38%
=======================================
Files 117 117
Lines 7358 7358
=======================================
Hits 6724 6724
Misses 634 634
☔ View full report in Codecov by Sentry. |
openfecli/commands/gather.py
Outdated
likelihood estimate of its absolute free, and the uncertainty in | ||
that. | ||
likelihood estimate of its absolute free, and the associated | ||
uncertainty from DDG replica averages and standard deviations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"derived from the network of" or something like that? Totally not past Balmer's peak
openfecli/commands/gather.py
Outdated
* 'dg' (default) reports the ligand and the results are the maximum | ||
likelihood estimate of its absolute free, and the uncertainty in | ||
that. | ||
* 'dg' (default) reports the ligand and the results as the maximum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @mikemhenry I overwrote this a bit, totally agreed it needed a change, just thought it might need a bit of an expansion on what you wrote.
@hannahbaumann does this seem sensible to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this generally sounds good, I was just a little bit confused about the "likelihood estimate of its absolute free" part and also the comma before ,and makes it look a little bit like the "from DDG replica averages and standard deviations" only belongs to the "uncertainty" part.
Maybe something like:
"* 'dg' (default) reports the ligand and its absolute free energy and the associated uncertainty as the maximum likelihood estimate obtained from DDG replica averages and standard deviations." Or maybe that's even more confusing, I'm not sure =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @hannahbaumann ! I've updated it accordingly
@hannahbaumann - I'm leaving this one in your hands to review / merge. |
Developers certificate of origin