Skip to content

Changed 'python -mpip' to 'python -m pip' for consistency #19739

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 21, 2021
Merged

Changed 'python -mpip' to 'python -m pip' for consistency #19739

merged 1 commit into from
Mar 21, 2021

Conversation

chbauman
Copy link
Contributor

The top-level files INSTALL.rst and README.rst both use the version with the space between the -m and the module in the command that is executed. To be more consistent, this was changed here as well.

PR Summary

PR Checklist

  • [N/A] Has pytest style unit tests (and pytest passes).
  • [N/A] Is Flake 8 compliant (run flake8 on changed files to check).
  • [N/A] New features are documented, with examples if plot related.
  • [N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • [N/A] Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • [N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@chbauman
Changed 'python -mpip' to 'python -m pip' for consistency
b37fe56 
The top-level files INSTALL.rst and README.rst both use the version with the space between the -m and the module that is execute. To be more consistent, this was changed here as well.
@tacaswell
Copy link
Member

It looks like we have a couple more:

✔ 18:10:46 $ ack 'python -m[^ ]' --rst
doc/devel/development_setup.rst
61:``python -mpip install -ve .``

doc/devel/contributing.rst
209:   python -mpip install coverage
210:   python -mpytest --cov=matplotlib --showlocals -v
214:   python -mpip install pyflakes

doc/faq/installing_faq.rst
59:   python -mpip install matplotlib

doc/faq/troubleshooting_faq.rst
164:    python -mpip install -v . > build.out

and for completeness all the places we do it the other way

✘ 18:43:00 $ ack 'python -m ' --rst
doc/devel/development_setup.rst
20:   python -m venv <file folder location>
53:    python -m pip install -ve .

doc/devel/documenting_mpl.rst
282:  python -m sphinx.ext.intersphinx 'https://docs.python.org/3/objects.inv'
283:  python -m sphinx.ext.intersphinx 'https://docs.scipy.org/doc/numpy/objects.inv'
284:  python -m sphinx.ext.intersphinx 'https://docs.scipy.org/doc/scipy/reference/objects.inv'
285:  python -m sphinx.ext.intersphinx 'https://pandas.pydata.org/pandas-docs/stable/objects.inv'

doc/devel/dependencies.rst
113:before running ``python -m pip install .``.

doc/users/prev_whats_new/github_stats_3.2.0.rst
425:* :ghpull:`15300`: CI: use python -m to make sure we are using the pip/pytest we want

README.rst
68:  python -m pytest

INSTALL.rst
18:  python -m pip install -U pip
19:  python -m pip install -U matplotlib
101:  python -m pip install -e .
106:  python -m pip install .
110:  python -m pip install -r requirements/dev/dev-requirements.txt

This has come up a couple times and I remember that people had strong opinions in either direction, but I do not remember what they are.

I suspect pro-' ' is easier to read / seperate the same of the module from the -m and I suspect the anti-' ' is visually separates arguments to Python from arguments to the module?

I tend to actually type no-space, but I think for docs the space is easier to read.

Thank you for working on this @chbauman and sorry you walked into something with some historical discussions behind it ;)

@tacaswell tacaswell added this to the v3.5.0 milestone Mar 18, 2021
tacaswell
tacaswell approved these changes Mar 18, 2021
View reviewed changes
Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

I have a weak preference for the other cases to be converted as well, but not going to block on that getting done.

@anntzer
Copy link
Contributor

anntzer commented Mar 19, 2021

I also tend to type with no space (because why bother), but I recently found out that if the module is a dotted name, then powershell gets confused if there's no space (see the point about -Dfoo.bar=baz at https://stackoverflow.com/questions/26614969/how-can-i-pass-batch-arguments-with-dots-into-a-powershell-script), so I think teaching to always use a space is better (as it works in more cases).

@jklymak jklymak merged commit a4a508b into matplotlib:master Mar 21, 2021
@jklymak jklymak mentioned this pull request Mar 21, 2021
Closed
@tacaswell
Copy link
Member

Thank you for your contribute @chbauman ! Congratulations on your first (I think) Matplotlib PR 🎉 hopefully we will hear from you again.

@chbauman
Copy link
Contributor Author

Thanks, was actually my second one, but it's also the first project I am contributing to. You will certainly hear more from me.

@tacaswell
Copy link
Member

@chbauman 🐑 sorry for my mistake, but still glad to have you helping!

@QuLogic QuLogic modified the milestones: v3.5.0, v3.4.0 Mar 25, 2021
@QuLogic
Copy link
Member

QuLogic commented Mar 25, 2021

@meeseeksdev backport to v3.4.x

@meeseeksmachine meeseeksmachine mentioned this pull request Mar 25, 2021
Merged
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Mar 25, 2021
@jklymak @meeseeksmachine
Backport PR matplotlib#19739: Changed 'python -mpip' to 'python -m pi…
887ec5f 
…p' for consistency
QuLogic added a commit that referenced this pull request Mar 25, 2021
@QuLogic
Merge pull request #19770 from meeseeksmachine/auto-backport-of-pr-19…
705123c 
…739-on-v3.4.x

Backport PR #19739 on branch v3.4.x (Changed 'python -mpip' to 'python -m pip' for consistency)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@chbauman @tacaswell @anntzer @QuLogic @jklymak