Analysis progmeter #892

kain88-de · 2016-07-07T13:26:51Z

Changes made in this Pull Request:

Adds progressmeter to AnalysisBase
less typying for an analysis

This command will print a progressbar and immediately run the alignment.

trj = AlignTrj(..., quiet=False).run()

Notice that the quiet parameter isn't explicitly added to the AlignTrj init function. Instead I catch it in the kwargs parameter that is passed to the AnalysisBase constructor. This is one of the few cases where I think it's OK to use a kwargs parameter instead of explicit typed parameters. As this allows us to add options to analysisbase that will be automatically picked up by any child-class.

PR Checklist

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

kain88-de · 2016-07-07T13:27:10Z

@richardjgowers any reason the progmeter wasn't in the base class already?

richardjgowers · 2016-07-07T20:10:19Z

No real reason, it would make a good addition for UI

On Thu, 7 Jul 2016, 14:27 Max Linke, notifications@github.com wrote:

@richardjgowers https://github.com/richardjgowers any reason the
progmeter wasn't in the base class already?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#892 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AI0jB83IozzqRwthestCvkjQyqeMfCe7ks5qTP6ugaJpZM4JHE0w
.

kain88-de · 2016-07-08T07:24:19Z

This should be done now. The docs could use an example of how to correctly derive from AnalysisBase.

One other thing this breaks user code who have written their own analysis based on AnalysisBase. The new setup requires that the init function is called to generate the _pm Progressmeter variable. I can check if I want a way to write this without breaking user-code, not sure how yet.

jbarnoud · 2016-07-08T08:03:13Z

I can check if I want a way to write this without breaking user-code, not sure how yet.

self.run could test for self._pm and self._quiet before calling _prepare, therefore knowing if a progress meter is available. It could then, set a dummy self._pm.echo if there is no progress meter.

An other solution would be to test for self._pm at each iteration, or to catch the call failure.

richardjgowers · 2016-07-08T08:04:42Z

Or setup the pm in _setup_frames which I think? Everyone uses

On Fri, 8 Jul 2016, 09:03 Jonathan Barnoud, notifications@github.com
wrote:

I can check if I want a way to write this without breaking user-code, not
sure how yet.

self.run could test for self._pm and self._quiet before calling _prepare,
therefore knowing if a progress meter is available. It could then, set a
dummy self._pm.echo if there is no progress meter.

An other solution would be to test for self._pm at each iteration, or to
catch the call failure.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#892 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AI0jBwzoFIJdl4LILJy4alv_jaB3QB5hks5qTgRBgaJpZM4JHE0w
.

kain88-de · 2016-07-08T08:25:18Z

The old API works and is unit-tested as well.

kain88-de · 2016-07-08T08:25:31Z

The docs need to be updated about that though.

richardjgowers · 2016-07-08T08:36:25Z

LGTM

kain88-de · 2016-07-08T08:38:07Z

@richardjgowers also not that it is now required to redefine _single_frame in a childclass to avoid triggering the NotImplementedError

I'll change the docs in the evening and then this can be merged.

coveralls · 2016-07-08T08:50:28Z

Coverage increased (+0.02%) to 80.403% when pulling 3c5f09b on kain88-de:analysis-progmeter into 5d78f03 on MDAnalysis:develop.

coveralls · 2016-07-08T09:19:40Z

Coverage increased (+0.008%) to 80.387% when pulling 3c5f09b on kain88-de:analysis-progmeter into 5d78f03 on MDAnalysis:develop.

coveralls · 2016-07-08T09:50:00Z

Coverage increased (+0.02%) to 80.403% when pulling 3c5f09b on kain88-de:analysis-progmeter into 5d78f03 on MDAnalysis:develop.

coveralls · 2016-07-08T10:25:01Z

Coverage increased (+0.008%) to 80.387% when pulling 3c5f09b on kain88-de:analysis-progmeter into 5d78f03 on MDAnalysis:develop.

kain88-de · 2016-07-11T14:17:12Z

Doc strings updated. Please have a look if I misspelled anything.

richardjgowers · 2016-07-11T14:18:18Z

package/CHANGELOG

+Enhancements
+
+  * All classes derived from 'AnalysisBase' now display a ProgressMeter
+  with 'quiet=False'


Shouldn't this be when quiet=True?

This is correct. You are thinking of verbose=True which is the normal way people turn on logging and output. For some reason MDAnalysis has decided in the past to rather go with the word quiet and there False would say the function can print and log stuff for me,

Ah right yeah, it's backwards to what I expect, ok cool

This also adds an initialization function to AnalysisBase. This is done now because we need to do more then just to setup the iteration.

Returning an axis object is considered to be better since the user is able to adjust the plot by himself if he wishes to do so.

Since user code might already exist we keep make sure the old API still works as well.

coveralls · 2016-07-11T14:46:05Z

Coverage increased (+0.02%) to 80.995% when pulling ffdf479 on kain88-de:analysis-progmeter into 4dcf071 on MDAnalysis:develop.

coveralls · 2016-07-11T15:16:33Z

Coverage increased (+0.02%) to 80.995% when pulling ffdf479 on kain88-de:analysis-progmeter into 4dcf071 on MDAnalysis:develop.

coveralls · 2016-07-11T15:49:56Z

Coverage increased (+0.02%) to 80.995% when pulling ffdf479 on kain88-de:analysis-progmeter into 4dcf071 on MDAnalysis:develop.

orbeckst · 2016-07-12T00:53:32Z

package/MDAnalysis/analysis/base.py

+    Afterwards the new analysis can be run like this.
+
+    .. code-block:: python
+       na = NewAnalysis(u.select_atoms('name CA'), 35).run()


I like this.

* PEP8 Changes * Add progressmeter to AnalysisBase This also adds an initialization function to AnalysisBase. This is done now because we need to do more then just to setup the iteration. * Adopt Contacts * Follow best practices for matplotlib Returning an axis object is considered to be better since the user is able to adjust the plot by himself if he wishes to do so. * Addopt Persistencelength * Adopt InterRDF * Adopt LinearDensity * Adopt AlignTrj * update changelog * Ensure old API still works Since user code might already exist we keep make sure the old API still works as well.

kain88-de added Component-Analysis Work in progress labels Jul 7, 2016

kain88-de force-pushed the analysis-progmeter branch 2 times, most recently from a711eaf to 1fe8352 Compare July 8, 2016 07:21

kain88-de changed the title ~~WIP: Analysis progmeter~~ Analysis progmeter Jul 8, 2016

kain88-de force-pushed the analysis-progmeter branch 3 times, most recently from 7ad1911 to 3c5f09b Compare July 8, 2016 08:36

kain88-de assigned richardjgowers Jul 8, 2016

kain88-de added this to the 0.16.0 milestone Jul 8, 2016

PEP8 Changes

1480ac7

kain88-de force-pushed the analysis-progmeter branch from 3c5f09b to f2d8d36 Compare July 11, 2016 14:16

richardjgowers reviewed Jul 11, 2016
View reviewed changes

kain88-de added 2 commits July 11, 2016 16:25

Add progressmeter to AnalysisBase

095f9b0

This also adds an initialization function to AnalysisBase. This is done now because we need to do more then just to setup the iteration.

Adopt AlignTrj

ba8c775

kain88-de added 6 commits July 11, 2016 16:25

Adopt Contacts

174042c

Addopt Persistencelength

4c90171

Adopt InterRDF

d55a4a0

Adopt LinearDensity

ba662ae

Follow best practices for matplotlib

9755a9f

Returning an axis object is considered to be better since the user is able to adjust the plot by himself if he wishes to do so.

update changelog

9edaa24

kain88-de force-pushed the analysis-progmeter branch from f2d8d36 to b08dcac Compare July 11, 2016 14:25

Ensure old API still works

ffdf479

Since user code might already exist we keep make sure the old API still works as well.

kain88-de force-pushed the analysis-progmeter branch from b08dcac to ffdf479 Compare July 11, 2016 14:28

kain88-de removed the Work in progress label Jul 11, 2016

orbeckst reviewed Jul 12, 2016
View reviewed changes

orbeckst mentioned this pull request Jul 12, 2016

change quiet keyword to verbose #903

Closed

2 tasks

richardjgowers merged commit 054a9de into MDAnalysis:develop Jul 14, 2016

richardjgowers mentioned this pull request Jul 18, 2016

Diffusion Maps rebase, new PR for final review #894

Merged

13 tasks

kain88-de mentioned this pull request Jul 30, 2016

write all base class API definition docs inside the class #919

Open

3 tasks

kain88-de deleted the analysis-progmeter branch August 16, 2016 14:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Analysis progmeter #892

Analysis progmeter #892

kain88-de commented Jul 7, 2016 •

edited

Loading

kain88-de commented Jul 7, 2016

richardjgowers commented Jul 7, 2016

kain88-de commented Jul 8, 2016

jbarnoud commented Jul 8, 2016

richardjgowers commented Jul 8, 2016

kain88-de commented Jul 8, 2016

kain88-de commented Jul 8, 2016

richardjgowers commented Jul 8, 2016

kain88-de commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

kain88-de commented Jul 11, 2016

richardjgowers Jul 11, 2016

kain88-de Jul 11, 2016

richardjgowers Jul 11, 2016

coveralls commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading

orbeckst Jul 12, 2016

Analysis progmeter #892

Analysis progmeter #892

Conversation

kain88-de commented Jul 7, 2016 • edited Loading

PR Checklist

kain88-de commented Jul 7, 2016

richardjgowers commented Jul 7, 2016

kain88-de commented Jul 8, 2016

jbarnoud commented Jul 8, 2016

richardjgowers commented Jul 8, 2016

kain88-de commented Jul 8, 2016

kain88-de commented Jul 8, 2016

richardjgowers commented Jul 8, 2016

kain88-de commented Jul 8, 2016 • edited Loading

coveralls commented Jul 8, 2016 • edited Loading

coveralls commented Jul 8, 2016 • edited Loading

coveralls commented Jul 8, 2016 • edited Loading

coveralls commented Jul 8, 2016 • edited Loading

kain88-de commented Jul 11, 2016

richardjgowers Jul 11, 2016

Choose a reason for hiding this comment

kain88-de Jul 11, 2016

Choose a reason for hiding this comment

richardjgowers Jul 11, 2016

Choose a reason for hiding this comment

coveralls commented Jul 11, 2016 • edited Loading

coveralls commented Jul 11, 2016 • edited Loading

coveralls commented Jul 11, 2016 • edited Loading

orbeckst Jul 12, 2016

Choose a reason for hiding this comment

kain88-de commented Jul 7, 2016 •

edited

Loading

kain88-de commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 8, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading

coveralls commented Jul 11, 2016 •

edited

Loading