Skip to content
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

Merged
merged 6 commits into from
Oct 11, 2023

Conversation

mikemhenry
Copy link
Contributor

@mikemhenry mikemhenry commented Sep 8, 2023

Developers certificate of origin

@mikemhenry
Copy link
Contributor Author

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (47cb562) 91.38% compared to head (a501dcd) 91.38%.
Report is 1 commits behind head on main.

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           
Files Coverage Δ
openfecli/commands/gather.py 85.98% <ø> (ø)

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

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.
Copy link
Member

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

* '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
Copy link
Member

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?

Copy link
Contributor

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 =)

Copy link
Member

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

@IAlibay
Copy link
Member

IAlibay commented Oct 11, 2023

@hannahbaumann - I'm leaving this one in your hands to review / merge.

@hannahbaumann hannahbaumann merged commit 4874b35 into main Oct 11, 2023
9 checks passed
@hannahbaumann hannahbaumann deleted the feat/tweak_help_msg_for_gather branch October 11, 2023 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants