Added `-j` shortcut for `--processes=` #7362

Merged
merged 5 commits into from Nov 8, 2016

Conversation

Projects
None yet
7 participants
Contributor

bcongdon commented Oct 30, 2016

Addresses #7361.

The argv parsing is a bit naive, but I'm not sure what matplotlib's opinion on getopt or argparse is.

Contributor

NelleV commented Oct 30, 2016

I am totally in favor of argparse!

@NelleV

NelleV approved these changes Oct 30, 2016

At some point we will have to refactor a bit this code, but for now this looks good.

tests.py
if __name__ == '__main__':
from matplotlib import default_test_modules, test
extra_args = []
- if '--no-pep8' in sys.argv:
+ parser = argparse.ArgumentParser()
+ parser.add_argument('--no-pep8', action="store_true")
@NelleV

NelleV Oct 30, 2016

Contributor

Do we really need no-pep8 and pep8? Seems they are redundant.

@efiring

efiring Oct 30, 2016

Owner

It's certainly confusing. I think the idea is that '--pep8' means do only PEP8 testing, '--no-pep8' means do everything except PEP8 testing, and leaving both out means test everything including PEP8.
When using argparse, it would be good to add 'help' kwargs to each add_argument call.

@NelleV

NelleV Oct 30, 2016

Contributor

oh wow… Thanks for the explanation.

Member

Kojoley commented Oct 30, 2016

I suppose argparse will interfere with nose on --help flag.

Contributor

anntzer commented Oct 30, 2016

Haven't looked at the PR itself but in any case you can prevent argparse from adding a --help with add_help=False (https://docs.python.org/3/library/argparse.html#add-help).

Contributor

NelleV commented Nov 5, 2016

Thanks @anntzer for pointing out the solution.
@bcongdon Do you think you can add this to the PR?

Contributor

bcongdon commented Nov 5, 2016 edited

Sure. Added it 👍

Owner

tacaswell commented Nov 8, 2016

This seems to have broken the --with-coverage flag https://travis-ci.org/matplotlib/matplotlib/jobs/173562893

See https://docs.python.org/3/library/argparse.html#nargs for REMAINDER. Any CL args we do not explicitly consume should fall through to nose / pytest (eventually).

tacaswell added this to the 2.0.1 (next bug fix release) milestone Nov 8, 2016

Owner

tacaswell commented Nov 8, 2016

milestoned as 2.0.1, but if the backport is at all hard, lets not bother.

Contributor

bcongdon commented Nov 8, 2016

Latest commit should fix passthrough flags like --with-coverage

tacaswell changed the title from Added `-j` shortcut for `--processes=` to [MRG+1] Added `-j` shortcut for `--processes=` Nov 8, 2016

bcongdon added some commits Oct 30, 2016

@bcongdon bcongdon Added `-j` shortcut for `--processes=` ca8dcee
@bcongdon bcongdon Refactored tests.py with `argparse` 5706876
@bcongdon bcongdon Add help texts for tests.py arguments dcb84d2
@bcongdon bcongdon Add add_help=False to argparser 76be64a
@bcongdon bcongdon Fix passthrough args like `—with-coverage`
7f29efe

@NelleV NelleV merged commit 3e89069 into matplotlib:master Nov 8, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 61.829%
Details
Contributor

NelleV commented Nov 8, 2016

@tacaswell This does not apply cleanly on v2.x. I don't think it is worth bothering with this patch for 2.0. What do you think?

Owner

efiring commented Nov 8, 2016

I'll answer: No, just leave it in master.

QuLogic changed the title from [MRG+1] Added `-j` shortcut for `--processes=` to Added `-j` shortcut for `--processes=` Nov 8, 2016

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