improved specifications for keyword arguments of run method

[amdnsr]

Fixes #3190

Changes made in this Pull Request:

• added kwargs in run method
• made verbose a part of kwargs before passing it to ProgressBar constructor

PR Checklist

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

[codecov]

Codecov Report

Merging #3191 (f5dbfbf) into develop (e32ce97) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head f5dbfbf differs from pull request most recent head 6834541. Consider uploading reports for the commit 6834541 to get more accurate results

@@           Coverage Diff            @@
##           develop    #3191   +/-   ##
========================================
  Coverage    92.73%   92.73%           
========================================
  Files          168      168           
  Lines        22683    22684    +1     
  Branches      3213     3213           
========================================
+ Hits         21035    21036    +1     
  Misses        1600     1600           
  Partials        48       48           

Impacted Files	Coverage Δ
package/MDAnalysis/analysis/base.py	95.50% <100.00%> (+0.05%)	⬆️

---

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 e32ce97...6834541. Read the comment docs.

[amdnsr]

I preferred not to delete the argument verbose, because this method might have been already used with verbose as the positional argument (fourth argument) i.e. without the keyword verbose.

[amdnsr]

Please provide me some feedback on the pull request.
Thank you

[IAlibay]

@amdnsr one of the @MDAnalysis/gsoc-mentors will review your PR as soon as they can. Reviews often take a few days as all developers on the project are volunteers, so we often have our own work priorities that come before MDAnalysis.

[lilyminium]

Thanks for looking at this, @amdnsr. Please add a test so you can know that your changes work. You should also document kwargs in the docstring and update CHANGELOG (message of what your fix is, and your GitHub username on top). As this is your first contribution to MDAnalysis, please also add yourself to AUTHORS.

[lilyminium]

May I ask why you didn't simply verbose=verbose, **kwargs? That way you don't have to modify the kwargs dictionary.

[amdnsr]

Hey, I tried that, and yes, I won't have to modify the kwargs dictionary in the run function of base.py file, but the kwargs dictionary in the __init__ function of log.py file is expecting verbose to be a key in its kwargs dictionary, but in the suggested way, verbose will be a named parameter, and not a key in the kwargs dictionary of the __init__ function.
So, I would have to instead do the modifications over there.
This modification seemed cleaner to me, thus, I did it this way, but if the suggested way (verbose=verbose, **kwargs) is preferred, I will do it that way instead.

[amdnsr]

Also, please guide me on creating the tests. From what I understand, I need to create a function at the end of the testsuite/MDAnalysisTests/analysis/test_base.py file, that calls the ProgressBar, with some kwargs, which will in turn be passed to tqdm, and consequently, affect the position of each individual ProgressBar.
But, I can't figure out how and what set of methods/functions to call to do it, and how to verify whether my function is behaving as expected.
Thanks

[lilyminium]

Have a look at the tqdm documentation for possible parameters you could pass through, and the existing ProgressBar tests in MDAnalysis for suggestions on how you could test for progress bar appearance. We also have a brief guide for writing tests in the user guide.

[lilyminium]

Hey, I tried that, and yes, I won't have to modify the kwargs dictionary in the run function of base.py file, but the kwargs dictionary in the __init__ function of log.py file is expecting verbose to be a key in its kwargs dictionary, but in the suggested way, verbose will be a named parameter, and not a key in the kwargs dictionary of the __init__ function.
So, I would have to instead do the modifications over there.

I don't think I understand what you mean here. Could you show me your suggested changes? If you just pass in verbose=verbose, **kwargs, the log.ProgressBar class will consider both of them to be in kwargs due to dictionary unpacking

[IAlibay]

@amdnsr the deadline for GSoC applications is rapidly approaching. Are you still interested in completing this?

[IAlibay]

There's already a 2.0.0 entry below, you should only need to add your entry there.

[amdnsr]

Apologies, I did not get it. Should I add my changes just above the following line?
09/05/19 IAlibay, richardjgowers

• 0.20.1

[IAlibay]

No, there's already an existing 2.0.0 entry just below, this is where it should go.

[IAlibay]

@amdnsr just a reminder that there's only a few days left before the GSoC deadline. You'll need to address @lilyminium's comments in order for this PR to progress.

[IAlibay]

Please make sure that you keep the empty line between the author list and the "External code" heading.

[IAlibay]

@amdnsr are you still looking to work on this?

[IAlibay]

Closing PR as stale.