Skip to content

Commit 87cf806

Browse files
committed
streamlined coding guide and added naming conventions table
1 parent 309981d commit 87cf806

File tree

4 files changed

+135
-113
lines changed

4 files changed

+135
-113
lines changed

doc/_static/mpl.css

+5
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,8 @@ div.wide-table table th.stub {
161161
font-style: italic;
162162
font-size: large;
163163
}
164+
165+
.checklist { list-style: none; padding: 0; margin: 0; }
166+
.checklist li { margin-left: 24px; padding-left: 23px; margin-right: 6px; }
167+
.checklist li:before { content: "\2610\2001"; margin-left: -24px; }
168+
.checklist li p {display: inline; }

doc/devel/coding_guide.rst

+111-113
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,3 @@
1-
.. raw:: html
2-
3-
<style>
4-
.checklist { list-style: none; padding: 0; margin: 0; }
5-
.checklist li { margin-left: 24px; padding-left: 23px; margin-right: 6px; }
6-
.checklist li:before { content: "\2610\2001"; margin-left: -24px; }
7-
.checklist li p {display: inline; }
8-
</style>
9-
101
.. _pr-guidelines:
112

123
***********************
@@ -17,144 +8,85 @@ Pull request guidelines
178
<https://docs.github.com/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests>`__
189
are the mechanism for contributing to Matplotlib's code and documentation.
1910

20-
It is recommended to check that your contribution complies with the following
21-
rules before submitting a pull request:
22-
23-
* If your pull request addresses an issue, please use the title to describe the
24-
issue (e.g. "Add ability to plot timedeltas") and mention the issue number
25-
in the pull request description to ensure that a link is created to the
26-
original issue (e.g. "Closes #8869" or "Fixes #8869"). This will ensure the
27-
original issue mentioned is automatically closed when your PR is merged. See
28-
`the GitHub documentation
29-
<https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue>`__
30-
for more details.
11+
We value contributions from people with all levels of experience. In particular,
12+
if this is your first PR not everything has to be perfect. We'll guide you
13+
through the PR process. Nevertheless, please try to follow our guidelines as well
14+
as you can to help make the PR process quick and smooth. If your pull request is
15+
incomplete or a work-in-progress, please mark it as a `draft pull requests <https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#draft-pull-requests>`_
16+
on GitHub and specify what feedback from the developers would be helpful.
3117

32-
* Formatting should follow the recommendations of PEP8_, as enforced by
33-
flake8_. Matplotlib modifies PEP8 to extend the maximum line length to 88
34-
characters. You can check flake8 compliance from the command line with ::
18+
Please be patient with reviewers. We try our best to respond quickly, but we have
19+
limited bandwidth. If there is no feedback within a couple of days, please ping
20+
us by posting a comment to your PR.
3521

36-
python -m pip install flake8
37-
flake8 /path/to/module.py
3822

39-
or your editor may provide integration with it. Note that Matplotlib
40-
intentionally does not use the black_ auto-formatter (1__), in particular due
41-
to its inability to understand the semantics of mathematical expressions
42-
(2__, 3__).
23+
Summary for pull request authors
24+
================================
4325

44-
.. _PEP8: https://www.python.org/dev/peps/pep-0008/
45-
.. _flake8: https://flake8.pycqa.org/
46-
.. _black: https://black.readthedocs.io/
47-
.. __: https://github.com/matplotlib/matplotlib/issues/18796
48-
.. __: https://github.com/psf/black/issues/148
49-
.. __: https://github.com/psf/black/issues/1984
26+
We recommend that you check that your contribution complies with the following
27+
guidelines before submitting a pull request:
5028

51-
* All public methods should have informative docstrings with sample usage when
52-
appropriate. Use the :ref:`docstring standards <writing-docstrings>`.
53-
54-
* For high-level plotting functions, consider adding a simple example either in
55-
the ``Example`` section of the docstring or the
56-
:ref:`examples gallery <gallery>`.
29+
.. rst-class:: checklist
5730

58-
* Changes (both new features and bugfixes) should have good test coverage. See
31+
* Changes, both new features and bugfixes, should have good test coverage. See
5932
:ref:`testing` for more details.
6033

61-
* Import the following modules using the standard scipy conventions::
62-
63-
import numpy as np
64-
import numpy.ma as ma
65-
import matplotlib as mpl
66-
import matplotlib.pyplot as plt
67-
import matplotlib.cbook as cbook
68-
import matplotlib.patches as mpatches
69-
70-
In general, Matplotlib modules should **not** import `.rcParams` using ``from
71-
matplotlib import rcParams``, but rather access it as ``mpl.rcParams``. This
72-
is because some modules are imported very early, before the `.rcParams`
73-
singleton is constructed.
74-
75-
* If your change is a major new feature, add an entry to the ``What's new``
76-
section by adding a new file in ``doc/users/next_whats_new`` (see
77-
:file:`doc/users/next_whats_new/README.rst` for more information).
78-
79-
* If you change the API in a backward-incompatible way, please document it in
80-
:file:`doc/api/next_api_changes/behavior`, by adding a new file with the
81-
naming convention ``99999-ABC.rst`` where the pull request number is followed
82-
by the contributor's initials. (see :file:`doc/api/api_changes.rst` for more
83-
information)
34+
* If you add a major new feature or change the API in a backward-incompatible
35+
way, please document it as described in :ref:`api_whats_new`
8436

8537
* If you add new public API or change public API, update or add the
8638
corresponding type hints. Most often this is found in the corresponding
8739
``.pyi`` file for the ``.py`` file which was edited. Changes in ``pyplot.py``
8840
are type hinted inline.
8941

90-
* See below for additional points about :ref:`keyword-argument-processing`, if
91-
applicable for your pull request.
92-
93-
.. note::
42+
* Code should follow our conventions as documented in our :ref:`coding-standards`
9443

95-
The current state of the Matplotlib code base is not compliant with all
96-
of these guidelines, but we expect that enforcing these constraints on all
97-
new contributions will move the overall code base quality in the right
98-
direction.
44+
* If applicable, see our guide to :ref:`keyword-argument-processing`.
9945

46+
* Update the :ref:`documentation <pr-documentation>` if necessary.
10047

101-
.. seealso::
102-
103-
* :ref:`coding_guidelines`
104-
* :ref:`testing`
105-
* :ref:`documenting-matplotlib`
48+
* All public methods should have informative docstrings with sample usage when
49+
appropriate. Use the :ref:`docstring standards <writing-docstrings>`.
10650

51+
* For high-level plotting functions, consider adding a simple example either in
52+
the ``Example`` section of the docstring or the :ref:`examples gallery <gallery>`.
10753

10854

109-
Summary for pull request authors
110-
================================
55+
When opening a pull request on Github, please ensure that:
11156

112-
.. note::
57+
.. rst-class:: checklist
11358

114-
* We value contributions from people with all levels of experience. In
115-
particular if this is your first PR not everything has to be perfect.
116-
We'll guide you through the PR process.
117-
* Nevertheless, please try to follow the guidelines below as well as you can to
118-
help make the PR process quick and smooth.
119-
* Be patient with reviewers. We try our best to respond quickly, but we
120-
have limited bandwidth. If there is no feedback within a couple of days,
121-
please ping us by posting a comment to your PR.
59+
* Changes were made on a :ref:`feature branch <make-feature-branch>`.
12260

123-
When making a PR, pay attention to:
61+
* :ref:`pre-commit <pre-commit-hooks>` checks for spelling, formatting, etc pass
12462

125-
.. rst-class:: checklist
63+
* The pull request targets the :ref:`main branch <pr-branch-selection>`
12664

127-
* :ref:`Target the main branch <pr-branch-selection>`.
128-
* Adhere to the :ref:`coding_guidelines`.
129-
* Update the :ref:`documentation <pr-documentation>` if necessary.
130-
* Aim at making the PR as "ready-to-go" as you can. This helps to speed up
131-
the review process.
132-
* It is ok to open incomplete or work-in-progress PRs if you need help or
133-
feedback from the developers. You may mark these as
134-
`draft pull requests <https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#draft-pull-requests>`_
135-
on GitHub.
136-
* When updating your PR, instead of adding new commits to fix something, please
137-
consider amending your initial commit(s) to keep the history clean.
138-
You can achieve this by using
65+
* If your pull request addresses an issue, please use the title to describe the
66+
issue (e.g. "Add ability to plot timedeltas") and mention the issue number
67+
in the pull request description to ensure that a link is created to the
68+
original issue (e.g. "Closes #8869" or "Fixes #8869"). This will ensure the
69+
original issue mentioned is automatically closed when your PR is merged. For more
70+
details, see `linking an issue and pull request <https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue>`__.
13971

140-
.. code-block:: bash
72+
* :ref:`pr-automated-tests` pass
14173

142-
git commit --amend --no-edit
143-
git push [your-remote-repo] [your-branch] --force-with-lease
74+
For guidance on creating and managing a pull request, please see
75+
:ref:`contributing`, :ref:`edit-flow`, and :ref:`update-pull-request`.
14476

145-
See also :ref:`contributing` for how to make a PR.
14677

14778
Summary for pull request reviewers
14879
==================================
14980

15081
.. redirect-from:: /devel/maintainer_workflow
15182

152-
.. note::
83+
**Please help review and merge PRs!**
84+
85+
If you have commit rights, then you are trusted to use them. Please be patient
86+
and `kind <https://youtu.be/tzFWz5fiVKU?t=49m30s>`__ with contributors.
15387

154-
* If you have commit rights, then you are trusted to use them.
155-
**Please help review and merge PRs!**
156-
* Be patient and `kind <https://youtu.be/tzFWz5fiVKU?t=49m30s>`__ with
157-
contributors.
88+
When reviewing, please ensure that the pull request satisfies the following
89+
requirements before merging it:
15890

15991
Content topics:
16092

@@ -187,6 +119,72 @@ Organizational topics:
187119
Detailed guidelines
188120
===================
189121

122+
.. _coding-standards:
123+
124+
Coding style guide
125+
------------------
126+
127+
While the current state of the Matplotlib code base is not compliant with all
128+
of these guidelines, but our goal in enforcing these constraints on new
129+
contributions is that it improves the readability and consistency of the code base
130+
going forward.
131+
132+
PEP8, as enforced by flake8
133+
^^^^^^^^^^^^^^^^^^^^^^^^^^^
134+
135+
Formatting should follow the recommendations of PEP8_, as enforced by flake8_.
136+
Matplotlib modifies PEP8 to extend the maximum line length to 88
137+
characters. You can check flake8 compliance from the command line with ::
138+
139+
python -m pip install flake8
140+
flake8 /path/to/module.py
141+
142+
or your editor may provide integration with it. Note that Matplotlib intentionally
143+
does not use the black_ auto-formatter (1__), in particular due to its inability
144+
to understand the semantics of mathematical expressions (2__, 3__).
145+
146+
.. _PEP8: https://www.python.org/dev/peps/pep-0008/
147+
.. _flake8: https://flake8.pycqa.org/
148+
.. _black: https://black.readthedocs.io/
149+
.. __: https://github.com/matplotlib/matplotlib/issues/18796
150+
.. __: https://github.com/psf/black/issues/148
151+
.. __: https://github.com/psf/black/issues/1984
152+
153+
154+
Package imports
155+
^^^^^^^^^^^^^^^
156+
Import the following modules using the standard scipy conventions::
157+
158+
import numpy as np
159+
import numpy.ma as ma
160+
import matplotlib as mpl
161+
import matplotlib.pyplot as plt
162+
import matplotlib.cbook as cbook
163+
import matplotlib.patches as mpatches
164+
165+
In general, Matplotlib modules should **not** import `.rcParams` using ``from
166+
matplotlib import rcParams``, but rather access it as ``mpl.rcParams``. This
167+
is because some modules are imported very early, before the `.rcParams`
168+
singleton is constructed.
169+
170+
Naming variables
171+
^^^^^^^^^^^^^^^^
172+
When feasible, please use our internal variable naming convention for objects
173+
of a given class and objects of any child class:
174+
175+
+---------------+------------------------------------+
176+
| variable name | base class |
177+
+===============+====================================+
178+
| ``fig`` | `~matplotlib.figure.FigureBase` |
179+
+---------------+------------------------------------+
180+
| ``ax`` | `~matplotlib.axes.Axes` |
181+
+---------------+------------------------------------+
182+
| ``trans`` | `~matplotlib.transforms.Transform` |
183+
+---------------+------------------------------------+
184+
185+
Generally, denote more than instance of the same class by adding numbers to
186+
the variable names, i.e. ``trans1``, ``trans2``.
187+
190188
.. _pr-documentation:
191189

192190
Documentation
@@ -205,7 +203,7 @@ Documentation
205203

206204
* See :ref:`documenting-matplotlib` for our documentation style guide.
207205

208-
.. _release_notes:
206+
.. _api_whats_new:
209207

210208
New features and API changes
211209
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

doc/devel/development_setup.rst

+2
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,8 @@ you are aware of the existing issues beforehand.
203203
* Run test cases to verify installation :ref:`testing`
204204
* Verify documentation build :ref:`documenting-matplotlib`
205205

206+
.. _pre-commit-hooks:
207+
206208
Install pre-commit hooks
207209
========================
208210
`pre-commit <https://pre-commit.com/>`_ hooks save time in the review process by

doc/devel/development_workflow.rst

+17
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,23 @@ If you don't think your request is ready to be merged, just say so in your pull
151151
request message and use the "Draft PR" feature of GitHub. This is a good way of
152152
getting some preliminary code review.
153153

154+
.. _update-pull-request:
155+
156+
Update a pull request
157+
=====================
158+
159+
When updating your pull request after making revisions, instead of adding new
160+
commits, please consider amending your initial commit(s) to keep the commit
161+
history clean.
162+
163+
You can achieve this by using
164+
165+
.. code-block:: bash
166+
167+
git commit --amend --no-edit
168+
git push [your-remote-repo] [your-branch] --force-with-lease
169+
170+
154171
Manage commit history
155172
=====================
156173

0 commit comments

Comments
 (0)