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

Docs for analysis modules should explain `verbose` #1630

Closed
kaceyreidy opened this Issue Aug 15, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@kaceyreidy
Contributor

kaceyreidy commented Aug 15, 2017

Modules in MDAnalysis.analysis should use the MDAnalysis progress bar.

EDIT (following the discussion below) — @orbeckst

  • analysis classes based on AnalysisBase should document that verbose enables the progress bar (and also explain what else is shown)
  • the default should possibly be verbose=True
  • changed issue title
@orbeckst

This comment has been minimized.

Show comment
Hide comment
@orbeckst

orbeckst Aug 15, 2017

Member

Which analysis classes in particular are missing the progress meter?

I am asking because anything that is already derived from AnalysisBase should have the progressmetere (when you use verbose=True).

Member

orbeckst commented Aug 15, 2017

Which analysis classes in particular are missing the progress meter?

I am asking because anything that is already derived from AnalysisBase should have the progressmetere (when you use verbose=True).

@kaceyreidy

This comment has been minimized.

Show comment
Hide comment
@kaceyreidy

kaceyreidy Aug 15, 2017

Contributor

PCA doesn't have a progress bar.

Contributor

kaceyreidy commented Aug 15, 2017

PCA doesn't have a progress bar.

@orbeckst

This comment has been minimized.

Show comment
Hide comment
@orbeckst

orbeckst Aug 15, 2017

Member

Did you try with verbose=True?

Member

orbeckst commented Aug 15, 2017

Did you try with verbose=True?

@kaceyreidy

This comment has been minimized.

Show comment
Hide comment
@kaceyreidy

kaceyreidy Aug 15, 2017

Contributor

Now I did.

Contributor

kaceyreidy commented Aug 15, 2017

Now I did.

@kaceyreidy kaceyreidy closed this Aug 15, 2017

@orbeckst

This comment has been minimized.

Show comment
Hide comment
@orbeckst

orbeckst Aug 15, 2017

Member

Should verbose=True be the default?

Member

orbeckst commented Aug 15, 2017

Should verbose=True be the default?

@kaceyreidy

This comment has been minimized.

Show comment
Hide comment
@kaceyreidy

kaceyreidy Aug 15, 2017

Contributor

I would say yes.

Contributor

kaceyreidy commented Aug 15, 2017

I would say yes.

@kaceyreidy

This comment has been minimized.

Show comment
Hide comment
@kaceyreidy

kaceyreidy Aug 15, 2017

Contributor

It should also be called something more obvious I think because I did not realize it would do that.

Contributor

kaceyreidy commented Aug 15, 2017

It should also be called something more obvious I think because I did not realize it would do that.

@kain88-de

This comment has been minimized.

Show comment
Hide comment
@kain88-de

kain88-de Aug 16, 2017

Member

Verbose is a common keyword used to enable additional reporting for a lot of scientific code and other programs. We like to stick to common idoms like that.

But the docs are not generated correctly on this class to show that the verbose keyword exists and what it means.

Member

kain88-de commented Aug 16, 2017

Verbose is a common keyword used to enable additional reporting for a lot of scientific code and other programs. We like to stick to common idoms like that.

But the docs are not generated correctly on this class to show that the verbose keyword exists and what it means.

@orbeckst orbeckst changed the title from Include progress bar in analysis modules to Docs for analysis modules should explain `verbose` Aug 16, 2017

@orbeckst

This comment has been minimized.

Show comment
Hide comment
@orbeckst

orbeckst Aug 16, 2017

Member

I edited the issue and re-open.

Member

orbeckst commented Aug 16, 2017

I edited the issue and re-open.

@orbeckst

This comment has been minimized.

Show comment
Hide comment
@orbeckst

orbeckst Nov 7, 2017

Member

@kaceyreidy do you want to make a PR for this issue?

Member

orbeckst commented Nov 7, 2017

@kaceyreidy do you want to make a PR for this issue?

@orbeckst orbeckst added the help wanted label Nov 7, 2017

@orbeckst orbeckst added this to the 1.0 milestone Jan 26, 2018

orbeckst added a commit that referenced this issue Mar 24, 2018

make diffusionmap.DistanceMatrix full y use AnalysisBase
Changes enable use of verbose (related to #1630)

orbeckst added a commit that referenced this issue Mar 24, 2018

analysis docs: explain verbose
- fixes #1630
- added explanation of `verbose` for all analysis classes that are derived from
  AnalysisBase

Note: The default remains False (as in AnalysisBase), unlike the request in the
      issue to set it to True. I didn't want to change the general behavior
      without further discussion but if there are more voices in favor of
      showing a progress bar by default then we can change to True.

@orbeckst orbeckst referenced this issue Mar 24, 2018

Merged

Doc fixes #1843

2 of 4 tasks complete

@orbeckst orbeckst self-assigned this Mar 27, 2018

@jbarnoud jbarnoud closed this in #1843 Mar 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment