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

Partial Fix: groups.py doctest error #4519

Closed
wants to merge 1 commit into from

Conversation

talagayev
Copy link
Contributor

@talagayev talagayev commented Mar 19, 2024

Partially addresses #3925 group.py doctest errors

Changes made in this Pull Request:

  • Doctest for groups.py. Five instances of errors occure in regards to difference between the expected output and obtained output during use of examples using .ix with the expected output dispaying dtype=int64 not present in obtained output. Example of previous expected output:

    • array([0, 1, 2], dtype=int64)
  • Corrected expected output according to the generated output as below:

    • array([0, 1, 2])

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

Developers certificate of origin


📚 Documentation preview 📚: https://mdanalysis--4519.org.readthedocs.build/en/4519/

The error lead to difference between expected and obtained output, since the obtained Ouput lacked dtype=int64, which was present in the expected Output this part was removed to make the expected Output fit the obtained Output
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 GitHub Discussions 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.

Copy link

Linter Bot Results:

Hi @talagayev! Thanks for making this PR. We linted your code and found the following:

There are currently no issues detected! 🎉

Copy link

codecov bot commented Mar 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.63%. Comparing base (0582265) to head (e2043b0).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4519      +/-   ##
===========================================
- Coverage    93.65%   93.63%   -0.03%     
===========================================
  Files          168      180      +12     
  Lines        21215    22294    +1079     
  Branches      3908     3908              
===========================================
+ Hits         19869    20875    +1006     
- Misses         888      961      +73     
  Partials       458      458              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orbeckst
Copy link
Member

This PR may be a duplicate of PR #4374 .

We'll first have to finalize the other PR and then we will see if there's anything else to be done.

@orbeckst orbeckst self-assigned this Mar 25, 2024
@talagayev
Copy link
Contributor Author

True, didn't notice PR #4374 , my bad.

The fixes in PR #4374 fix all the error messages regarding doctests in group.py.

Should i close thus this PR?

@orbeckst
Copy link
Member

Leave it open for now. I can close it if/when the other PR gets merged.

@orbeckst
Copy link
Member

Given that the prior PR #4374 was merged and addresses the original issue, I am just closing this PR. Thanks for the work @talagayev , sorry that it's not going in.

@orbeckst orbeckst closed this Mar 30, 2024
@talagayev
Copy link
Contributor Author

Given that the prior PR #4374 was merged and addresses the original issue, I am just closing this PR. Thanks for the work @talagayev , sorry that it's not going in.

It's fine, my mistake that i didn't notice that PR #4374 already adressed the issue.

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.

None yet

2 participants