Skip to content

kill off getslice - #977

Merged
dwhswenson merged 7 commits into
openpathsampling:dev-2.0from
sroet:kill_getslice
Feb 21, 2021
Merged

kill off getslice#977
dwhswenson merged 7 commits into
openpathsampling:dev-2.0from
sroet:kill_getslice

Conversation

@sroet

@sroet sroet commented Feb 19, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

As list.__getslice__() has been deprecated since python 2.0 and killed since 3.0 this also removes the Trajectory override of it.

@codecov

codecov Bot commented Feb 19, 2021 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #977 (ed62e0d) into dev-2.0 (767d358) will decrease coverage by 0.24%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           dev-2.0     #977      +/-   ##
===========================================
- Coverage    80.25%   80.01%   -0.25%     
===========================================
  Files          136      136              
  Lines        14449    14445       -4     
===========================================
- Hits         11596    11558      -38     
- Misses        2853     2887      +34
Impacted Files Coverage Δ
openpathsampling/engines/trajectory.py 73.11% <ø> (-0.71%) ⬇️
openpathsampling/ensemble.py 84.55% <100.00%> (+0.04%) ⬆️
openpathsampling/netcdfplus/netcdfplus.py 74.95% <100.00%> (-0.05%) ⬇️
openpathsampling/netcdfplus/stores/named.py 67.03% <0.00%> (-3.92%) ⬇️
openpathsampling/netcdfplus/dictify.py 61.97% <0.00%> (-2.64%) ⬇️
openpathsampling/deprecations.py 97.46% <0.00%> (-2.54%) ⬇️
openpathsampling/netcdfplus/stores/attribute.py 84.21% <0.00%> (-1.76%) ⬇️
openpathsampling/netcdfplus/cache.py 59.44% <0.00%> (-1.75%) ⬇️
...enpathsampling/analysis/shooting_point_analysis.py 96.85% <0.00%> (-1.58%) ⬇️
openpathsampling/netcdfplus/base.py 76.55% <0.00%> (-1.38%) ⬇️
... and 6 more

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 767d358...adf571f. Read the comment docs.

@sroet

sroet commented Feb 19, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

@dwhswenson Do we want to keep python 2.7 support alive here (I don't have a python 2 environment anymore)? If so, feel free to close this PR

@dwhswenson

dwhswenson commented Feb 19, 2021

Copy link
Copy Markdown
Member

Our official commitment is to support Py27 until
OPS 2.0. This is to allow PhD students who started in 2.7 to finish with working code. (Hoping that's mainly @arjunwadhawan, who has now submitted his thesis!) Could you close and target this against dev-2.0 instead?

@sroet
sroet changed the base branch from master to dev-2.0 February 19, 2021 19:52
@sroet

sroet commented Feb 19, 2021

Copy link
Copy Markdown
Member Author

@dwhswenson Github allows to change the base branch 😄 , so this should not be correct

@sroet

sroet commented Feb 19, 2021

Copy link
Copy Markdown
Member Author

do you want me to properly rebase onto dev-2.0, or don't you mind pulling the master/dev-2.0 diffs as well?

@dwhswenson

dwhswenson commented Feb 19, 2021

Copy link
Copy Markdown
Member

Doesn't matter. I've been doing the occasional merge of master into dev-2.0. If I've fallen a bit behind, no worries if your PR catches me up. As long as contributors get the appropriate credit (which (1) they would with this; (2) not an issue in this specific case) then I'm happy.

@dwhswenson dwhswenson added 2.0 issues and PRs for the 2.0 release drop-py2 labels Feb 20, 2021
dwhswenson
dwhswenson approved these changes Feb 21, 2021
View reviewed changes

@dwhswenson dwhswenson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. There's a drop in coverage largely due to removing Py2.7 from the testing matrix (some code paths no longer used). I'll follow this up with a PR removing some of those code paths.

Related: note that part of the roadmap to 2.0 includes modifying trajectory so that it no longer inherits directly from list (instead wrapping a list and inheriting from collections.abc.Sequence).

@dwhswenson
dwhswenson merged commit 6ff2cbb into openpathsampling:dev-2.0 Feb 21, 2021
@dwhswenson dwhswenson mentioned this pull request Feb 21, 2021
Merged
@sroet
sroet deleted the kill_getslice branch February 21, 2021 11:51
@sroet sroet mentioned this pull request Jul 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dwhswenson dwhswenson dwhswenson approved these changes

Assignees

No one assigned

Labels

2.0 issues and PRs for the 2.0 release drop-py2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sroet @dwhswenson