-
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
Raise ValueError for conflicting pH and H+ #205
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #205 +/- ##
==========================================
+ Coverage 75.70% 81.68% +5.98%
==========================================
Files 9 9
Lines 1494 1496 +2
Branches 256 257 +1
==========================================
+ Hits 1131 1222 +91
+ Misses 317 227 -90
- Partials 46 47 +1 ☔ View full report in Codecov by Sentry. |
Thank you @gnuhpdiem this looks good to me. Could you please also add another branch to the conditional that logs a warning if Note that you should use After that, please add tests for both branches here. You can see an example of how to capture a warning message in the tests here. Finally, please install |
Thanks for your detailed review! I'll update the PR soon with the requested changes! |
This looks terrific @gnuhpdiem , thank you! I believe the test failures are due to an unrelated dependency update, which I will address soon. Once I do that, I'll rerun the tests and as long as they pass, this is ready to merge. |
Summary
Major changes:
Checklist
ruff
. (For guidance in fixing rule violates, see rule list)mypy
.Tip: Install
pre-commit
hooks to auto-check types and linting before every commit: