Skip to content

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

Merged
jklymak merged 1 commit intomatplotlib:masterfrom
chbauman:master
Mar 21, 2021
Merged

Changed 'python -mpip' to 'python -m pip' for consistency#19739
jklymak merged 1 commit intomatplotlib:masterfrom
chbauman:master

Conversation

@chbauman
Copy link
Copy Markdown
Contributor

@chbauman chbauman commented Mar 18, 2021

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
Copy Markdown
Member

tacaswell commented Mar 18, 2021

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

tacaswell commented Mar 22, 2021

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

@chbauman
Copy link
Copy Markdown
Contributor Author

chbauman commented Mar 22, 2021

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
Copy Markdown
Member

tacaswell commented Mar 22, 2021

@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
Copy Markdown
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