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

Set expected list of elements for Parmed PSF test #4016

Merged
merged 5 commits into from
Jan 30, 2023
Merged

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Jan 30, 2023

Fixes #4015

Changes made in this Pull Request:

  • Add a list of expected elements rather than just empty strings

PR Checklist

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

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Base: 93.52% // Head: 93.52% // No change to project coverage 👍

Coverage data is based on head (c7b0707) compared to base (d73523f).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4016   +/-   ##
========================================
  Coverage    93.52%   93.52%           
========================================
  Files          190      190           
  Lines        25028    25028           
  Branches      3542     3542           
========================================
  Hits         23407    23407           
  Misses        1100     1100           
  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.

@IAlibay IAlibay changed the title [wip] Set expected list of elements for Parmed PSF test Set expected list of elements for Parmed PSF test Jan 30, 2023
@IAlibay
Copy link
Member Author

IAlibay commented Jan 30, 2023

@MDAnalysis/coredevs can I get a quick check here - this issue is making CI fail across the board.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

lgtm

Is there anything in the docs that we need to add about new capabilities?

Do we need to restrict to newer versions of parmed or is the new capability just incidental and it doesn't really matter to us?

@IAlibay
Copy link
Member Author

IAlibay commented Jan 30, 2023

lgtm

Is there anything in the docs that we need to add about new capabilities?

Do we need to restrict to newer versions of parmed or is the new capability just incidental and it doesn't really matter to us?

@orbeckst It's purely incidental & only affects tests - i.e. upstream now supports it so if you use the latest parmed you'll get elements instead of empty strings. Otherwise you'll get empty strings (i.e. the expected behaviour before the previous release).

I've clarified this in the changelog.

@IAlibay IAlibay merged commit c7c5f0a into develop Jan 30, 2023
@IAlibay IAlibay deleted the fix-parmed-psf branch January 30, 2023 19:19
IAlibay added a commit that referenced this pull request Mar 29, 2023
Fixes #4015
* fix expected list of elements for Parmed PSF test
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.

New parmed release causes PSF tests to fail
2 participants