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

Generalize continuous check in chain trajectory #3849

Closed
wants to merge 19 commits into from

Conversation

jaclark5
Copy link
Contributor

@jaclark5 jaclark5 commented Sep 22, 2022

Fixes #3546

Changes made in this Pull Request:

  • Added a fix this issue with a base.ReaderBase method.

Note that the overwritten method in the LAMMPS reader was a solution that was integrated into my personal working version with very ugly if statements. I think a solution like this will keep things clean.

PR Checklist

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

@codecov
Copy link

codecov bot commented Nov 5, 2022

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (58bbc44) 93.60% compared to head (e21f45f) 93.60%.

❗ Current head e21f45f differs from pull request most recent head de0fdcb. Consider uploading reports for the commit de0fdcb to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3849   +/-   ##
========================================
  Coverage    93.60%   93.60%           
========================================
  Files          193      193           
  Lines        25146    25152    +6     
  Branches      4055     4055           
========================================
+ Hits         23538    23544    +6     
  Misses        1092     1092           
  Partials       516      516           
Impacted Files Coverage Δ
package/MDAnalysis/coordinates/LAMMPS.py 95.06% <100.00%> (+0.03%) ⬆️
package/MDAnalysis/coordinates/base.py 94.60% <100.00%> (+0.01%) ⬆️
package/MDAnalysis/coordinates/chain.py 86.69% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jaclark5 jaclark5 marked this pull request as ready for review November 5, 2022 13:33
@github-actions
Copy link

github-actions bot commented Apr 2, 2023

Linter Bot Results:

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

Some issues were found with the formatting of your code.

Code Location Outcome
main package ⚠️ Possible failure
testsuite ⚠️ Possible failure

Please have a look at the darker-main-code and darker-test-code steps here for more details: https://github.com/MDAnalysis/mdanalysis/actions/runs/5281931368/jobs/9556168994


Please note: The black linter is purely informational, you can safely ignore these outcomes if there are no flake8 failures!

@jaclark5
Copy link
Contributor Author

jaclark5 commented Apr 6, 2023

@MDAnalysis/coredevs I'm curious to know your thoughts on this change

@jaclark5 jaclark5 closed this Jun 22, 2023
@jaclark5 jaclark5 deleted the stitch_traj_duplicate branch June 22, 2023 15:12
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.

continuous chain reader support for LAMMPS trajectories
2 participants