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

Fixes issues 3166 and 3255 #3256

Merged
merged 4 commits into from
May 4, 2021
Merged

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented May 2, 2021

Fixes #3166 #3255

Changes made in this Pull Request:

  • Changes NAMDBIN.py permissions back to rw only.
  • Fixes single frame dt calls for netcdf files.
  • Adds warning to TRZ _get_dt() call
    • Note: inconsistent setting of dt compared to base, needs changing in 2.1.0?

PR Checklist

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

@pep8speaks
Copy link

pep8speaks commented May 2, 2021

Hello @IAlibay! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-05-02 18:44:26 UTC

@IAlibay IAlibay marked this pull request as ready for review May 2, 2021 18:43
@@ -330,6 +330,9 @@ def _get_dt(self):
t1 = self.ts.time
dt = t1 - t0
except StopIteration:
msg = ('dt information could not be obtained, defaulting to 0 ps. '
'Note: in MDAnalysis 2.1.0 this default will change 1 ps.')
warnings.warn(msg)
return 0
Copy link
Member Author

Choose a reason for hiding this comment

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

This is inconsistent with what we do in base.py, ideally this should just throw an AttributeError and default on what's done in base?

@codecov
Copy link

codecov bot commented May 2, 2021

Codecov Report

Merging #3256 (b3f2eb8) into develop (fe22dc3) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3256      +/-   ##
===========================================
+ Coverage    92.85%   92.86%   +0.01%     
===========================================
  Files          170      170              
  Lines        22840    22845       +5     
  Branches      3240     3240              
===========================================
+ Hits         21208    21215       +7     
+ Misses        1584     1582       -2     
  Partials        48       48              
Impacted Files Coverage Δ
package/MDAnalysis/coordinates/NAMDBIN.py 100.00% <ø> (ø)
package/MDAnalysis/coordinates/TRJ.py 97.71% <100.00%> (+0.01%) ⬆️
package/MDAnalysis/coordinates/TRZ.py 89.33% <100.00%> (+0.81%) ⬆️

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 fe22dc3...b3f2eb8. Read the comment docs.

@IAlibay IAlibay added this to the 2.0 milestone May 3, 2021
@richardjgowers richardjgowers merged commit 79183bf into MDAnalysis:develop May 4, 2021
@IAlibay IAlibay deleted the issue-3166 branch August 25, 2021 00:46
@IAlibay IAlibay added the defect label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NCDFReader cannot get dt for single frame trajectories
3 participants