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

[WIP] Changes holetraj start/stop/step keywords (Issue #2513) #2514

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Feb 7, 2020

Fixes #2513
Towards #2443

Changes made in this Pull Request:

  • Removes support for start/stop/step in :meth:HOLEtraj.__init__.
  • Adds a check_slice_indices to the values passed to :meth:HOLEtraj.run.

PR Checklist

  • Tests? (Don't have HOLE on my machine, relying on Travis in this instance)
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@codecov
Copy link

codecov bot commented Feb 7, 2020

Codecov Report

Merging #2514 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2514   +/-   ##
========================================
  Coverage    90.49%   90.49%           
========================================
  Files          170      170           
  Lines        23355    23355           
  Branches      3077     3077           
========================================
  Hits         21136    21136           
  Misses        1623     1623           
  Partials       596      596

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 fcf8e23...0067e92. Read the comment docs.

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.

Looks pretty good , thank you. There's an issue with kwargs, which needs to be documented (versionchanged and CHANGELOG) and fixed.

Please also add CHANGELOG entry.

package/MDAnalysis/analysis/hole.py Show resolved Hide resolved
package/MDAnalysis/analysis/hole.py Outdated Show resolved Hide resolved
@IAlibay IAlibay requested a review from orbeckst February 10, 2020 08:39
@orbeckst orbeckst merged commit 6fa3b6d into MDAnalysis:develop Feb 13, 2020
@IAlibay IAlibay deleted the holtraj-kwargs branch June 6, 2020 10:21
@fiona-naughton fiona-naughton added maintainability Component-Analysis deprecation Deprecated functionality to give advance warning for API changes. labels Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component-Analysis deprecation Deprecated functionality to give advance warning for API changes. maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove start/stop/step keywords from HOLEtraj __init__
3 participants