Issue #2395 AtomGroup.guess_bonds by AHMED-salah00 · Pull Request #4059 · MDAnalysis/mdanalysis

AHMED-salah00 · 2023-03-09T05:55:29Z

Fixes #2395

Changes made in this Pull Request:

I have added fudge_factor and lower_bound to AtomGorup.guess_bonds() method. Also added test_universe_guess_bonds_arguments() test function AtomGroup.guess_bonds misleading documentation, could pass in fudge_factor #2395 (comment)

PR Checklist

Docs?
CHANGELOG updated?
Issue raised/referenced?

I have added fudge_factor and lower_bound to AtomGorup.guess_bonds()

codecov · 2023-03-09T06:11:38Z

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (72e3cdb) 93.56% compared to head (bae5d67) 93.56%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4059   +/-   ##
========================================
  Coverage    93.56%   93.56%           
========================================
  Files          191      191           
  Lines        25083    25083           
  Branches      4050     4050           
========================================
  Hits         23470    23470           
  Misses        1093     1093           
  Partials       520      520

Impacted Files	Coverage Δ
package/MDAnalysis/core/groups.py	`97.51% <100.00%> (ø)`
package/MDAnalysis/core/universe.py	`97.26% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

orbeckst · 2023-03-10T01:31:35Z

@ianmkenney could you please look after this PR? Thanks!

package/MDAnalysis/core/groups.py

ianmkenney

Apart from minor formatting issues, this looks good! @AHMED-salah00 can you also update the CHANGELOG and add yourself the AUTHORS file if you haven't contributed before?

IAlibay

Thanks, this is a good start. I would like to see maybe one additional test to ensure the parameters are passing through fine, if possible.

package/MDAnalysis/core/groups.py

package/MDAnalysis/core/universe.py

package/MDAnalysis/core/groups.py

Co-authored-by: Ian Kenney <ianmkenney@users.noreply.github.com>

…h00/mdanalysis into issue-MDAnalysis#2395

package/MDAnalysis/core/universe.py

package/MDAnalysis/core/groups.py

package/MDAnalysis/core/universe.py

testsuite/MDAnalysisTests/core/test_universe.py

ianmkenney

Simple formatting change and use fixtures for testing different parameter sets.

IAlibay

Just a couple of extra review comments on top of the things @ianmkenney raised.

package/MDAnalysis/core/groups.py

testsuite/MDAnalysisTests/core/test_universe.py

package/MDAnalysis/core/groups.py

testsuite/MDAnalysisTests/core/test_universe.py

ianmkenney

I think this looks pretty good, just need to add a comment for the if statement. @IAlibay, any final thoughts?

testsuite/MDAnalysisTests/core/test_universe.py

ianmkenney

LGTM. Can you resolve the merge conflicts with develop?

added args to guess_bonds

31d1b43

I have added fudge_factor and lower_bound to AtomGorup.guess_bonds()

github-actions bot added the Component-Core label Mar 9, 2023

Update groups.py

f82836f

AHMED-salah00 force-pushed the issue-#2395 branch from c37570b to f82836f Compare March 9, 2023 11:03

Update the documentations

778e5ca

orbeckst assigned ianmkenney Mar 10, 2023

ianmkenney reviewed Mar 10, 2023

View reviewed changes

package/MDAnalysis/core/groups.py Outdated Show resolved Hide resolved

ianmkenney reviewed Mar 10, 2023

View reviewed changes

package/MDAnalysis/core/groups.py Outdated Show resolved Hide resolved

ianmkenney suggested changes Mar 10, 2023

View reviewed changes

AHMED-salah00 and others added 2 commits March 11, 2023 03:53

modify CHANGELOG .

c8aeb69

Merge branch 'develop' into issue-MDAnalysis#2395

d46120a

AHMED-salah00 requested a review from ianmkenney March 11, 2023 02:04

Update CHANGELOG

28e3fef

IAlibay requested changes Mar 13, 2023

View reviewed changes

package/MDAnalysis/core/groups.py Show resolved Hide resolved

package/MDAnalysis/core/groups.py Show resolved Hide resolved

AHMED-salah00 added 2 commits March 13, 2023 19:08

version update

3c14253

Added tests for fudge_factor & lower_bond attributes

f8b727b

AHMED-salah00 requested review from IAlibay and removed request for ianmkenney March 14, 2023 06:57

ianmkenney reviewed Mar 14, 2023

View reviewed changes

package/MDAnalysis/core/universe.py Outdated Show resolved Hide resolved

ianmkenney reviewed Mar 14, 2023

View reviewed changes

package/MDAnalysis/core/groups.py Outdated Show resolved Hide resolved

AHMED-salah00 and others added 3 commits March 14, 2023 17:30

Update package/MDAnalysis/core/universe.py

084c27b

Co-authored-by: Ian Kenney <ianmkenney@users.noreply.github.com>

Update

36b4beb

Merge branch 'issue-MDAnalysis#2395' of https://github.com/AHMED-sala…

102ee49

…h00/mdanalysis into issue-MDAnalysis#2395

AHMED-salah00 commented Mar 15, 2023

View reviewed changes

package/MDAnalysis/core/universe.py Outdated Show resolved Hide resolved

IAlibay reviewed Mar 15, 2023

View reviewed changes

package/MDAnalysis/core/groups.py Outdated Show resolved Hide resolved

AHMED-salah00 requested review from IAlibay and ianmkenney and removed request for IAlibay March 15, 2023 13:48

AHMED-salah00 and others added 3 commits March 16, 2023 15:45

add tests for atomgroup.guess_bonds()

1fd3546

Merge branch 'develop' into issue-MDAnalysis#2395

bb5053f

Update groups.py

dca1c54

ianmkenney reviewed Mar 17, 2023

View reviewed changes

package/MDAnalysis/core/universe.py Outdated Show resolved Hide resolved

ianmkenney reviewed Mar 17, 2023

View reviewed changes

testsuite/MDAnalysisTests/core/test_universe.py Outdated Show resolved Hide resolved

ianmkenney suggested changes Mar 17, 2023

View reviewed changes

IAlibay requested changes Mar 17, 2023

View reviewed changes

AHMED-salah00 added 3 commits March 18, 2023 09:03

Update tests implementations and fix some docs.

2cc8f7c

Update test_universe.py

dd29f45

Update groups.py

5de497b

AHMED-salah00 requested a review from IAlibay March 18, 2023 07:21

IAlibay reviewed Mar 18, 2023

View reviewed changes

testsuite/MDAnalysisTests/core/test_universe.py Show resolved Hide resolved

AHMED-salah00 requested a review from ianmkenney March 19, 2023 15:43

check_atomgroup only in case of default parameter values

590fb89

ianmkenney reviewed Mar 22, 2023

View reviewed changes

testsuite/MDAnalysisTests/core/test_universe.py Show resolved Hide resolved

ianmkenney suggested changes Mar 22, 2023

View reviewed changes

Added a comment for the if statement.

40f6034

IAlibay reviewed Mar 22, 2023

View reviewed changes

testsuite/MDAnalysisTests/core/test_universe.py Show resolved Hide resolved

AHMED-salah00 requested a review from ianmkenney March 22, 2023 19:17

ianmkenney suggested changes Mar 22, 2023

View reviewed changes

Merge branch 'develop' into issue-MDAnalysis#2395

bae5d67

AHMED-salah00 requested a review from ianmkenney March 23, 2023 15:10

IAlibay approved these changes Mar 23, 2023

View reviewed changes

ianmkenney approved these changes Mar 23, 2023

View reviewed changes

ianmkenney merged commit 09b8b16 into MDAnalysis:develop Mar 23, 2023

AHMED-salah00 deleted the issue-#2395 branch March 23, 2023 16:46

IAlibay added the defect label Sep 21, 2023

github-staff deleted a comment from AHMED-salah00 Oct 23, 2024

Conversation

AHMED-salah00 commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

codecov bot commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

orbeckst commented Mar 10, 2023

Uh oh!

Uh oh!

Uh oh!

ianmkenney left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ianmkenney left a comment

Choose a reason for hiding this comment

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ianmkenney left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ianmkenney left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AHMED-salah00 commented Mar 9, 2023 •

edited

Loading

codecov bot commented Mar 9, 2023 •

edited

Loading

ianmkenney left a comment •

edited

Loading