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

Remove ProgressMeter #2743

Merged
merged 6 commits into from
Jun 13, 2020
Merged

Remove ProgressMeter #2743

merged 6 commits into from
Jun 13, 2020

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Jun 11, 2020

Towards #2739

Changes made in this Pull Request:

  • Removes deprecated ProgressMeter

Questions:

  • Do we need a versionchanged in lib.log?

PR Checklist

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

@codecov
Copy link

codecov bot commented Jun 11, 2020

Codecov Report

Merging #2743 into develop will decrease coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2743      +/-   ##
===========================================
- Coverage    91.30%   91.25%   -0.06%     
===========================================
  Files          176      176              
  Lines        24007    23967      -40     
  Branches      3159     3153       -6     
===========================================
- Hits         21920    21871      -49     
- Misses        1459     1470      +11     
+ Partials       628      626       -2     
Impacted Files Coverage Δ
...ckage/MDAnalysis/analysis/hbonds/hbond_analysis.py 80.39% <ø> (ø)
package/MDAnalysis/analysis/density.py 89.02% <100.00%> (-0.04%) ⬇️
package/MDAnalysis/lib/log.py 76.78% <100.00%> (-19.01%) ⬇️

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 105e47a...08ad2c6. Read the comment docs.

@IAlibay IAlibay marked this pull request as ready for review June 13, 2020 06:33
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.

I would say we add a versionchanged to the module docs to indicate that the class was removed. If nothing else it will help people to go back to 1.0 if they really can’t change their scripts. The way I see it, is that in this PR it is really easy to add the information, it will be hard and tedious if we later decide to do it or if we need to keep answer questions.

@@ -80,12 +80,11 @@
.. autogenerated, see Online Docs

"""
from __future__ import print_function, division, absolute_import
from __future__ import absolute_import

Copy link
Member

Choose a reason for hiding this comment

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

Remove?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I'll remove all the py2 stuff from lib at the same time.

@orbeckst
Copy link
Member

And yay, -282 lines of code!

Btw, if there’s widespread support for not adding versionchanged for dropped content then say so. I am just thinking that the technical docs can be technical and have useful information. Anything we put in the docs now will hopefully allow users to come to their own conclusions as opposed to us explaining.

@IAlibay
Copy link
Member Author

IAlibay commented Jun 13, 2020

And yay, -282 lines of code!

Btw, if there’s widespread support for not adding versionchanged for dropped content then say so. I am just thinking that the technical docs can be technical and have useful information. Anything we put in the docs now will hopefully allow users to come to their own conclusions as opposed to us explaining.

I think my main thing was that I wasn't too sure where the versionchanged should go, I've just placed one under "13.2.6.2. Other functions and classes for logging purposes" hopefully it looks reasonable.

@richardjgowers richardjgowers merged commit bf74d65 into MDAnalysis:develop Jun 13, 2020
@IAlibay IAlibay deleted the remove-pm branch June 13, 2020 09:19
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this pull request Mar 30, 2021
* removes PM from package

* Remove PM tests

* updates CHANGELOG

* Remove tests

* Adds docstring versionchange
@fiona-naughton fiona-naughton added maintainability deprecation Deprecated functionality to give advance warning for API changes. labels Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Deprecated functionality to give advance warning for API changes. maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants