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

Updated test_encore.py #4032

Merged
merged 10 commits into from Feb 28, 2023
Merged

Updated test_encore.py #4032

merged 10 commits into from Feb 28, 2023

Conversation

v-parmar
Copy link
Contributor

Fixed #3695

Updated test_encore.py file.
Updated 'rmsd' attribute with 'results.rmsd' and 'rmsf' with 'results.rmsf' in test_encore.py file.

Updated 'rmsd' attribute with 'results.rmsd' and 'rmsf' with 'results.rmsf'.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello there first time contributor! Welcome to the MDAnalysis community! We ask that all contributors abide by our Code of Conduct and that first time contributors introduce themselves on the developer mailing list so we can get to know you. You can learn more about participating here. Please also add yourself to package/AUTHORS as part of this PR.

@codecov
Copy link

codecov bot commented Feb 20, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4e3ea0e) 93.53% compared to head (861e1e7) 93.53%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4032   +/-   ##
========================================
  Coverage    93.53%   93.53%           
========================================
  Files          191      191           
  Lines        25063    25063           
  Branches      4042     4042           
========================================
  Hits         23443    23443           
  Misses        1099     1099           
  Partials       521      521           

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@RMeli RMeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR @v-parmar. As you can see, the "linters" checks are not successful. You need to format the code properly in order for them to pass.

Also, since you are a first time contributor, you need to add your name to the package/AUTHORS file, as well as your GitHub handle to package/CHANGELOG.

Finally, this PR solves the issue for test_encore.py. Have you checked if the same happens for other tests, as suggested by @orbeckst in #3695 (comment)?

testsuite/MDAnalysisTests/analysis/test_encore.py Outdated Show resolved Hide resolved
@RMeli RMeli self-assigned this Feb 26, 2023
@RMeli
Copy link
Member

RMeli commented Feb 27, 2023

Thanks @v-parmar. There are now conflicts in package/CHANGELOG that need to be solved.

Finally, this PR solves the issue for test_encore.py. Have you checked if the same happens for other tests, as suggested by @orbeckst in #3695 (comment)?

Can you please confirm if you have checked this?

@v-parmar
Copy link
Contributor Author

Thanks @v-parmar. There are now conflicts in package/CHANGELOG that need to be solved.

Finally, this PR solves the issue for test_encore.py. Have you checked if the same happens for other tests, as suggested by @orbeckst in #3695 (comment)?

Can you please confirm if you have checked this?

Yes, I have checked.

@RMeli
Copy link
Member

RMeli commented Feb 28, 2023

@v-parmar unfortunately the way you changed the CHANGELOG is not quite right. You need to add your GitHub handle alongside the handles of other developers, for the 2.5.0 release. The ??/??/?? is just a placeholder for the date of the release, which will be updated during the release process.

Please move your GitHub handle and the description of the change to the appropriate place. Only the description under "Fixes" is needed for this PR.

You will likely get back the conflicts you were having before, that you will need to fix. You can use the GitHub Web interface to solve conflicts. See addressing merge conflicts for more details.

Copy link
Member

@RMeli RMeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @v-parmar , LGTM!

package/CHANGELOG Outdated Show resolved Hide resolved
@richardjgowers richardjgowers merged commit de6d0a8 into MDAnalysis:develop Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests should use results.rmsf to avoid DeprecationWarning
4 participants