-
Notifications
You must be signed in to change notification settings - Fork 7
Feat/ruff fix fmt #120
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
Feat/ruff fix fmt #120
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #120 +/- ##
==========================================
- Coverage 98.23% 98.18% -0.05%
==========================================
Files 14 13 -1
Lines 678 661 -17
==========================================
- Hits 666 649 -17
Misses 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
atravitz
left a comment
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.
Just a couple things!
src/kartograf/atom_mapping_scorer.py
Outdated
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.
Looks like this is causing the docs to fail - could you remove this in the docs as well? And add to the 2.0 changelog!
| from .metric_shape_difference import ( | ||
| MappingShapeMismatchScorer as MappingShapeMismatchScorer, | ||
| ) | ||
| from .metric_shape_difference import ( |
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.
same as above, I (subjectively) prefer the prior formatting.
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.
not sure if this changed or my opinion changed, but i'm fine with this now 😄
| beta = 1000 / (const.k * const.Avogadro * T) | ||
| V = k_hook * (rmsd - accepted_distance_rmsd) | ||
| p = np.exp(-beta * V) if (np.exp(-beta * V) < 1) else 1 | ||
| p = min(1, np.exp(-beta * V)) |
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.
🚀
|
Looks like changes in OpenFreeEnergy/openfe-benchmarks@23f2c94 are causing an issue, not sure why pinning the commit hash didn't fix it |
atravitz
left a comment
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.
lgtm!
| from .metric_shape_difference import ( | ||
| MappingShapeMismatchScorer as MappingShapeMismatchScorer, | ||
| ) | ||
| from .metric_shape_difference import ( |
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.
not sure if this changed or my opinion changed, but i'm fine with this now 😄
|
|
||
| import functools | ||
| from typing import Callable | ||
| from collections.abc import Callable |
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.
RIP typing
| "ANN", # type annotations https://pypi.org/project/flake8-annotations/ | ||
| "E", # pycodestyle errors | ||
| "ERA", # find commented-out code https://pypi.org/project/eradicate/ | ||
| "F", # Pyflakes | ||
| "FURB", # A tool for refurbishing and modernizing Python codebases https://github.com/dosisod/refurb | ||
| "I", # isort | ||
| # "C901", # mccabe complexity TODO: add this back in | ||
| "S", # security related lint https://pypi.org/project/flake8-bandit/ | ||
| "UP", # pyupgrade | ||
| "W", # pycodestyle warnings | ||
| "YTT", # detect issues using sys to check python version https://pypi.org/project/flake8-2020/ | ||
| ] |
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.
excited about many of these - I'll add some to konnektor and gufe!
|
we need openfe for testing the notebook, just need to make sure that CI uninstalls the kartograf that gets pulled in |
Sweet that works |
|
No API break detected ✅ |
Checklist
newsentryDevelopers certificate of origin