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

Fix GSD Windows compatibility #2384

Merged

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Oct 30, 2019

Fixes #1923.

Changes made in this Pull Request:

  • Add installation requirement of gsd>=1.9.3 on Windows.
  • Removed conditional failures when attempting to read GSD files on Windows.
  • Updated tests so they won't skip GSD on Windows.

Note that gsd>=1.8.0 requires Python 3, so I was careful not to invalidate Python 2 compatibility on non-Windows platforms. This should be fine in terms of compatibility because Python 3 is already a requirement for MDAnalysis on Windows (citation: #2116, #2093 (comment)). The Appveyor tests were updated to version 1.9.3 for consistency.

As a new developer to MDAnalysis, I appreciate the effort put into making the development guide and style guides. However, it took me a while to find the development guide in the GitHub wiki. I would recommend including the Guide for Developers in the CONTRIBUTING file (the Style Guide is already linked, but that's only a subset of the Developer Guide in my understanding) and/or as a link on the MDAnalysis website, with a heading like "Developer Guide."

PR Checklist

  • Tests? (tests are no longer skipped)
  • Docs? (no updates needed)
  • CHANGELOG updated?
  • Issue raised/referenced?

@bdice bdice marked this pull request as ready for review October 30, 2019 04:45
@bdice bdice changed the title Fix GSD windows compatibility Fix GSD Windows compatibility Oct 30, 2019
@codecov
Copy link

codecov bot commented Oct 30, 2019

Codecov Report

Merging #2384 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2384      +/-   ##
===========================================
+ Coverage    89.92%   89.93%   +<.01%     
===========================================
  Files          175      175              
  Lines        21916    21912       -4     
  Branches      2876     2874       -2     
===========================================
- Hits         19709    19707       -2     
  Misses        1612     1612              
+ Partials       595      593       -2
Impacted Files Coverage Δ
package/MDAnalysis/topology/GSDParser.py 91.66% <100%> (+1.66%) ⬆️
package/MDAnalysis/coordinates/GSD.py 86.66% <100%> (+1.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc67acf...3d6fb3c. Read the comment docs.

@richardjgowers richardjgowers merged commit 2501d3e into MDAnalysis:develop Oct 30, 2019
IAlibay added a commit that referenced this pull request Dec 18, 2023
@IAlibay IAlibay mentioned this pull request Dec 18, 2023
5 tasks
orbeckst added a commit that referenced this pull request Dec 21, 2023
Update of AUTHORS and CHANGELOG with inferred author contributions.

*  Removed duplicate mattwthompson in 0.20.0 changelog entry.: mattwthompson was placed twice by accident, this removes this duplication.

* Addition of missing authors.

   An retrospective analysis of the authors found via `git shortlog -s -n --email --branches="develop"` found several commits by authors which were not present in the `AUTHORS.md` file.

    - Zhenbo Li: commited via PR: Started tests for gnm. #803 and Make Travis run tests on OSX. #771,
    - Jenna M. Swarthout via PR Update CoC according to suggestions from current CoC committee #4289 and Point to new reporting form link (owned by conduct@mdanalysis.org) #4298,
    - Bradley Dice via PR   Fix GSD Windows compatibility #2384 ,
    - David Minh via PR #2668

   There seemed to be no indications in the above mentioned PRs that the author did not want to be in the authors file, it looks like it was just forgotten.

* Addition of missing entries from the changelog

   Continued cross referencing of the git shortlog output but also accounting for the changelog identified several individuals that had not been included in the changelog entries for the release they contributed under. They were added to the relevant entry of the changelog based on the merge commit date.

   Please note that for Tone Bengsten, we a) had no github handle (so they were assigned @tbengtsen), and b) no specific commit. Given we know that this individual was including alongside the encore merge, they were assigned to the 0.16.0 release.

* Update CHANGELOG
* PR #1218
* PR #1284 and #1408
* PR #4109
* based on git history
* PRs #803 and #771 (author addition, changelog addition)
* PR #2255 and #2221
* PR #1225
* PR #4289 and #4298
* PR #4031
* PR #4085
* PR #3635
* PR #2356
* PR #2559
* No GH handle - Encore author addition @tbengtsen
* PR #4184
* PR #2614
* PR #2219
* PR #2384
* PR #2668
* Add missing entry for Jenna

Thanks to @fiona-naughton for helping out with digging into this data :)

Co-authored-by: Fiona Naughton <fiona@mdanalysis.org>
Co-authored-by: Oliver Beckstein <orbeckst@mdanalysis.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GSD & Windows
2 participants