-
Notifications
You must be signed in to change notification settings - Fork 59
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
add more element valence #169
Conversation
Aunity
commented
Sep 21, 2023
- Fix some bugs #302 PROPKA fails with "AttributeError: 'NoneType' object has no attribute 'group_type'" #140
- Add more element valence
Sorry, I am travelling/at conferences so reviewing this PR may take a while. Please feel free to ignore me if I am not able to review in time.
… On Sep 23, 2023, at 11:31 AM, Nathan Baker ***@***.***> wrote:
@sobolevnrm <https://github.com/sobolevnrm> requested your review on: #169 <#169> add more element valence.
|
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.
Please reference your issue #163 from the description.
Tests would be great. Here is a simple test for issue #140
: speleo3@8e5121a
@Aunity do you need help with the changes I proposed? |
@speleo3 Sorry, it's been busy lately. Now it's fixed, please check again. |
Getting close, thanks @Aunity for the updates. There is a syntax error that needs fixing, see https://github.com/jensengroup/propka/actions/runs/6492041705/job/17653467393?pr=169 |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #169 +/- ##
==========================================
- Coverage 73.08% 73.05% -0.04%
==========================================
Files 25 25
Lines 3972 3989 +17
==========================================
+ Hits 2903 2914 +11
- Misses 1069 1075 +6
☔ View full report in Codecov by Sentry. |
Thanks for the final fix. Please don't reformat entire files in your pull requests. It adds noise and controversy to your valuable contribution. In my opinion, if reformatting is desired, it should be discussed with the development team first and done in a separate pull request, and ideally be implemented with a commit hook or CI job which enforces the new style in the future. |
@speleo3 Thank you for reminding me. I have revoked the previous commit and made some modifications again. |