verbose not set in analysis.AnalysisBase.run

[lilyminium]

Expected behavior

From #1463, I think that AnalysisBase.run(verbose=True) is supposed to override the existing self._verbose. From #2206, it seems that verbose is supposed to be deprecated, but there are no deprecation warnings about it. (For what it's worth, I think the progress meter is cute.)

EDIT (@orbeckst): Analysis classes should be able to take the verbose kwarg for both the __init__() constructor and the run() method. The value give for run() should override the constructor value while run() is executing, as described in #1463 .

Actual behavior

Nothing happens.

Code to reproduce the behavior

mdanalysis/package/MDAnalysis/analysis/base.py

Line 187 in 9d5520c

verbose = getattr(self, '_verbose', False) if verbose is None else verbose

Nothing's done with verbose. _setup_frames defaults to False.

mdanalysis/package/MDAnalysis/analysis/base.py

Line 149 in 9d5520c

verbose = getattr(self, '_verbose', False)

Currently version of MDAnalysis

• Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)") 0.20.1
• Which version of Python (python -V)? 3.7
• Which operating system? MacOS

[orbeckst]

@lilyminium thanks for pointing out this inconsistency.

From #2206, it seems that verbose is supposed to be deprecated, but there are no deprecation warnings about it.

I just re-read my comments namely #2206 (comment) and as far as I can tell, I mis-remembered: verbose in both AnalysisBase.__init__() and AnalysisBase.run() is not deprecated but conforms to the "Bauhaus" API #719 and #1463 is correct. (I updated my comment.)

[Purva-Chaudhari]

@lilyminium , @orbeckst I would like to work on the issue

[lilyminium]

Please go ahead @Purva-Chaudhari! Put up a pull request and link this issue, and it will get reviewed.

[kpiyush04]

Hi! @lilyminium @orbeckst
I would like to work on this issue

[Purva-Chaudhari]

@lilyminium From what I have understood(ran example with run(verbose=True)),in the run method in AnalysisBase() self_verbose is not getting overridden. Referring to issue #2206 I Raised deprecation warning if step/start/stop is none and made their scope private. Is it now expected that the self_verbose in run to override the default .

[Purva-Chaudhari]

If I am mistaken in understanding the objective please do let me know

[lilyminium]

@kpiyush04 thanks for looking at this, but as @Purva-Chaudhari has already started working on the issue, their pull request will get prioritised. Perhaps you could work on something else, e.g. #2599 was recently opened.

@Purva-Chaudhari Yep, that sounds about right -- as @orbeckst clarified above, verbose is not deprecated. Setting verbose in run() should override the existing self._verbose.

[orbeckst]

Just to add: @Purva-Chaudhari please open a PR that references this issue in a timely manner. Just saying "I want to do this" is not enough – we'll give you some leeway but we expect that you follow up within a day or so.

[Purva-Chaudhari]

@orbeckst Sure will follow that

[Purva-Chaudhari]

@lilyminium could you please review the PR, I am a bit new to open source contribution. Hope i made it in a correct way. If not please do let me know and also regarding the changes I made, are they correct or what should I improve in the code,
Thanking you

[lilyminium]

@Purva-Chaudhari Thanks for having a go at this, welcome to open source! I see that you opened a PR in your own fork of MDAnalysis -- however, to merge code into this code-base, you need to open one here. The user guide lays out the steps of how to contribute to MDAnalysis, including adding and writing tests.

You have also commented which part of the code you would like to change, but you should actually make changes to the code so that tests can be run on it to see if it works. In terms of your suggested change -- it's not quite what we were looking for. The line verbose = getattr(self, '_verbose', False) overrides the verbose argument passed in run, whereas the value I pass into run() should override the saved _verbose, at least while the class is running.

Please ping me or another dev for a proper review when you open a PR into this repository :-)