Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added -j shortcut for --processes= #7362

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

Merged
NelleV merged 5 commits into matplotlib:master from bcongdon:multi-process-flag
Nov 8, 2016

Conversation

Copy link
Contributor

@bcongdon 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.

Copy link
Member

NelleV commented Oct 30, 2016

I am totally in favor of argparse!

Copy link
Member

@NelleV NelleV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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



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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow... Thanks for the explanation.

Copy link
Member

Kojoley commented Oct 30, 2016

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

Copy link
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).

Copy link
Member

NelleV commented Nov 5, 2016

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

Copy link
Contributor Author

bcongdon commented Nov 5, 2016
edited
Loading

Sure. Added it 👍

Copy link
Member

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 tacaswell added this to the 2.0.1 (next bug fix release) milestone Nov 8, 2016
Copy link
Member

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

Copy link
Contributor Author

bcongdon commented Nov 8, 2016

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

@tacaswell tacaswell changed the title (削除) Added -j shortcut for --processes= (削除ここまで) (追記) [MRG+1] Added -j shortcut for --processes= (追記ここまで) Nov 8, 2016
@NelleV NelleV merged commit 3e89069 into matplotlib:master Nov 8, 2016
Copy link
Member

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?

Copy link
Member

efiring commented Nov 8, 2016

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

tacaswell reacted with thumbs up emoji

@QuLogic QuLogic modified the milestones: 2.1 (next point release), 2.0.1 (next bug fix release) Nov 8, 2016
@QuLogic QuLogic changed the title (削除) [MRG+1] Added -j shortcut for --processes= (削除ここまで) (追記) 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
Reviewers

@efiring efiring efiring left review comments

@NelleV NelleV NelleV approved these changes

@tacaswell tacaswell tacaswell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.1
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /