Skip to content

Commit ffcfc29

Browse files
authored
Contributing guidelines improvements (#495)
* add stub CONTRIBUTING.md * add new contributing guidelines * clean up errors, typos * review feedback * add documentation section
1 parent ee03ed4 commit ffcfc29

File tree

5 files changed

+222
-28
lines changed

5 files changed

+222
-28
lines changed

.github/CONTRIBUTING.md

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Contributing
2+
============
3+
4+
We welcome your contributions! Please see the [contributing](http://pvlib-python.readthedocs.io/en/latest/contributing.html) page for information about how to contribute.

.github/PULL_REQUEST_TEMPLATE.md

+9-7
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
11
pvlib python pull request guidelines
22
====================================
33

4-
Thank you for your contribution to pvlib python!
4+
Thank you for your contribution to pvlib python! You may delete all of these instructions except for the list below.
55

6-
You may submit a pull request with your code at any stage of completion, however, before the code can be merged the following items must be addressed:
6+
You may submit a pull request with your code at any stage of completion.
7+
8+
The following items must be addressed before the code can be merged. Please don't hesitate to ask for help if you're unsure of how to accomplish any of the items below:
79

810
- [ ] Closes issue #xxxx
9-
- [ ] Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests must pass on the TravisCI and Appveyor testing services.
10-
- [ ] Code quality and style is sufficient. Passes ``git diff upstream/master -u -- "*.py" | flake8 --diff`` and/or landscape.io linting service.
11-
- [ ] New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
11+
- [ ] I am familiar with the [contributing guidelines](http://pvlib-python.readthedocs.io/en/latest/contributing.html).
12+
- [ ] Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests (usually) must pass on the TravisCI and Appveyor testing services.
1213
- [ ] Updates entries to `docs/sphinx/source/api.rst` for API changes.
1314
- [ ] Adds description and name entries in the appropriate `docs/sphinx/source/whatsnew` file for all changes.
14-
15-
Please don't hesitate to ask for help if you're unsure of how to accomplish any of the above. You may delete all of these instructions except for the list above.
15+
- [ ] Code quality and style is sufficient. Passes ``git diff upstream/master -u -- "*.py" | flake8 --diff``
16+
- [ ] New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
17+
- [ ] Pull request is nearly complete and ready for detailed review.
1618

1719
Brief description of the problem and proposed solution (if not already fully described in the issue linked to above):

docs/sphinx/source/contributing.rst

+206-17
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,27 @@ Encouraging more people to help develop pvlib-python is essential to our
77
success. Therefore, we want to make it easy and rewarding for you to
88
contribute.
99

10+
There is a lot of material in this section, aimed at a variety of
11+
contributors from novice to expert. Don't worry if you don't (yet)
12+
understand parts of it.
13+
1014

1115
Easy ways to contribute
1216
~~~~~~~~~~~~~~~~~~~~~~~
1317

14-
Here are a few ideas for you can contribute, even if you are new to
18+
Here are a few ideas for how you can contribute, even if you are new to
1519
pvlib-python, git, or Python:
1620

21+
* Ask and answer `pvlib questions on StackOverflow <http://stackoverflow.com/questions/tagged/pvlib>`_
22+
and participate in discussions in the `pvlib-python google group <https://groups.google.com/forum/#!forum/pvlib-python>`_.
1723
* Make `GitHub issues <https://github.com/pvlib/pvlib-python/issues>`_
18-
and contribute to the conversation about how to resolve them.
24+
and contribute to the conversations about how to resolve them.
1925
* Read issues and pull requests that other people created and
2026
contribute to the conversation about how to resolve them.
27+
Look for issues tagged with
28+
`good first issue <https://github.com/pvlib/pvlib-python/labels/good%20first%20issue>`_,
29+
`easy <https://github.com/pvlib/pvlib-python/labels/easy>`_,
30+
or `help wanted <https://github.com/pvlib/pvlib-python/labels/help%20wanted>`_.
2131
* Improve the documentation and the unit tests.
2232
* Improve the IPython/Jupyter Notebook tutorials or write new ones that
2333
demonstrate how to use pvlib-python in your area of expertise.
@@ -35,6 +45,9 @@ pvlib-python, git, or Python:
3545
How to contribute new code
3646
~~~~~~~~~~~~~~~~~~~~~~~~~~
3747

48+
The basics
49+
----------
50+
3851
Contributors to pvlib-python use GitHub's pull requests to add/modify
3952
its source code. The GitHub pull request process can be intimidating for
4053
new users, but you'll find that it becomes straightforward once you use
@@ -53,16 +66,11 @@ process. Here's an outline of the process:
5366
request.
5467

5568
The Pandas project maintains an excellent `contributing page
56-
<https://github.com/pydata/pandas/wiki/Contributing>`_ that goes into
57-
detail on each of these steps. Also see GitHub's `Set Up Git
69+
<http://pandas.pydata.org/pandas-docs/stable/contributing.html>`_ that goes
70+
into detail on each of these steps. Also see GitHub's `Set Up Git
5871
<https://help.github.com/articles/set-up-git/>`_ and `Using Pull
5972
Requests <https://help.github.com/articles/using-pull-requests/>`_.
6073

61-
Note that you do not need to make all of your changes before creating a
62-
pull request. Your pull requests will automatically be updated when you
63-
commit new changes and push them to GitHub. This gives everybody an easy
64-
way to comment on the code and can make the process more efficient.
65-
6674
We strongly recommend using virtual environments for development.
6775
Virtual environments make it trivial to switch between different
6876
versions of software. This `astropy guide
@@ -73,17 +81,187 @@ environment.
7381

7482
You must include documentation and unit tests for any new or improved
7583
code. We can provide help and advice on this after you start the pull
76-
request.
84+
request. See the Testing section below.
85+
86+
87+
.. _pull-request-scope:
88+
89+
Pull request scope
90+
------------------
91+
92+
This section can be summed up as "less is more".
93+
94+
A pull request can quickly become unmanageable if too many lines are
95+
added or changed. "Too many" is hard to define, but as a rule of thumb,
96+
we encourage contributions that contain less than 50 lines of primary code.
97+
50 lines of primary code will typically need at least 250 lines
98+
of documentation and testing. This is about the limit of what the
99+
maintainers can review on a regular basis.
100+
101+
A pull request can also quickly become unmanageable if it proposes
102+
changes to the API in order to implement another feature. Consider
103+
clearly and concisely documenting all proposed API changes before
104+
implementing any code. Modifying
105+
`api.rst <https://github.com/pvlib/pvlib-python/blob/master/docs/sphinx/source/api.rst>`_
106+
and/or the latest `whatsnew file <https://github.com/pvlib/pvlib-python/tree/master/docs/sphinx/source/whatsnew>`_
107+
can help formalize this process.
108+
109+
Questions about related issues frequently come up in the process of
110+
addressing implementing code for a pull request. Please try to avoid
111+
expanding the scope of your pull request (this also applies to
112+
reviewers!). We'd rather see small, well-documented additions to the
113+
project's technical debt than see a pull request languish because its
114+
scope expanded beyond what the reviewer community is capable of
115+
processing.
116+
117+
Of course, sometimes it is necessary to make a large pull request. We
118+
only ask that you take a few minutes to consider how to break it into
119+
smaller chunks before proceeding.
120+
121+
pvlib-python contains :ref:`3 "layers" of code <modeling-paradigms>`:
122+
functions, PVSystem/Location, and ModelChain. We recommend that
123+
contributors focus their work on only one or two of those layers in a
124+
single pull request. New models are *not* required to be available to
125+
the higher-level API!
126+
127+
128+
When should I submit a pull request?
129+
------------------------------------
130+
131+
The short answer: anytime.
132+
133+
The long answer: it depends. If in doubt, go ahead and submit. You do
134+
not need to make all of your changes before creating a pull request.
135+
Your pull requests will automatically be updated when you commit new
136+
changes and push them to GitHub.
137+
138+
There are pros and cons to submitting incomplete pull-requests. On the
139+
plus side, it gives everybody an easy way to comment on the code and can
140+
make the process more efficient. On the minus side, it's easy for an
141+
incomplete pull request to grow into a multi-month saga that leaves
142+
everyone unhappy. If you submit an incomplete pull request, please be
143+
very clear about what you would like feedback on and what we should
144+
ignore. Alternatives to incomplete pull requests include creating a
145+
`gist <https://gist.github.com>`_ or experimental branch and linking to
146+
it in the corresponding issue.
147+
148+
The best way to ensure that a pull request will be reviewed and merged in
149+
a timely manner is to:
150+
151+
#. Start by creating an issue. The issue should be well-defined and
152+
actionable.
153+
#. Ask the maintainers to tag the issue with the appropriate milestone.
154+
#. Make a limited-scope pull request. It can be a lot of work to check all of
155+
the boxes in `pull request guidelines
156+
<https://github.com/pvlib/pvlib-python/blob/master/.github/PULL_REQUEST_TEMPLATE.md>`_,
157+
especially for pull requests with a lot of new primary code.
158+
See :ref:`pull-request-scope`.
159+
#. Tag pvlib community members or ``@pvlib/maintainer`` when the pull
160+
request is ready for review. (see :ref:`pull-request-reviews`)
161+
162+
163+
.. _pull-request-reviews:
164+
165+
Pull request reviews
166+
--------------------
167+
168+
The pvlib community and maintainers will review your pull request in a
169+
timely fashion. Please "ping" ``@pvlib/maintainer`` if it seems that
170+
your pull request has been forgotten at any point in the pull request
171+
process.
172+
173+
Keep in mind that the PV modeling community is diverse and each pvlib
174+
community member brings a different perspective when reviewing code.
175+
Some reviewers bring years of expertise in the sub-field that your code
176+
contributes to and will focus on the details of the algorithm. Other
177+
reviewers will be more focused on integrating your code with the rest of
178+
pvlib, ensuring that it is feasible to maintain, that it meets the
179+
:ref:`code style <code-style>` guidelines, and that it is
180+
:ref:`comprehensively tested <testing>`. Limiting the scope of the pull
181+
request makes it much more likely that all of these reviews can be
182+
conducted and any issues can be resolved in a timely fashion.
183+
184+
Sometimes it's hard for reviewers to be immediately available, so the
185+
right amount of patience is to be expected. That said, interested
186+
reviewers should do their best to not wait until the last minute to put
187+
in their two cents.
188+
189+
190+
.. _code-style:
191+
192+
Code style
193+
~~~~~~~~~~
194+
195+
pvlib python generally follows the `PEP 8 -- Style Guide for Python Code
196+
<https://www.python.org/dev/peps/pep-0008/>`_. Maximum line length for code
197+
is 79 characters.
198+
199+
Code must be compatible with python 2.7 and 3.4+.
200+
201+
pvlib python uses a mix of full and abbreviated variable names. See
202+
:ref:`variables_style_rules`. We could be better about consistency.
203+
Prefer full names for new contributions. This is especially important
204+
for the API. Abbreviations can be used within a function to improve the
205+
readability of formulae.
206+
207+
Set your editor to strip extra whitespace from line endings. This
208+
prevents the git commit history from becoming cluttered with whitespace
209+
changes.
210+
211+
Please see :ref:`Documentation` for information specific to documentation
212+
style.
213+
214+
Remove any ``logging`` calls and ``print`` statements that you added
215+
during development. ``warning`` is ok.
216+
217+
We typically use GitHub's
218+
"`squash and merge` <https://help.github.com/articles/about-pull-request-merges/#squash-and-merge-your-pull-request-commits>_"
219+
feature to merge your pull request into pvlib. GitHub will condense the
220+
commit history of your branch into a single commit when merging into
221+
pvlib-python/master (the commit history on your branch remains
222+
unchanged). Therefore, you are free to make commits that are as big or
223+
small as you'd like while developing your pull request.
224+
225+
226+
.. _documentation:
77227

78-
The maintainers will follow same procedures, rather than making direct
79-
commits to the main repo. Exceptions may be made for extremely minor
80-
changes, such as fixing documentation typos.
228+
Documentation
229+
~~~~~~~~~~~~~
81230

231+
Documentation must be written in
232+
`numpydoc format <https://numpydoc.readthedocs.io/>`_.
233+
234+
The numpydoc format includes a specification for the allowable input
235+
types. Python's `duck typing <https://en.wikipedia.org/wiki/Duck_typing>`_
236+
allows for multiple input types to work for many parameters. pvlib uses
237+
the following generic descriptors as short-hand to indicate which
238+
specific types may be used:
239+
240+
* dict-like : dict, OrderedDict, pd.Series
241+
* numeric : scalar, np.array, pd.Series. Typically int or float dtype.
242+
* array-like : np.array, pd.Series. Typically int or float dtype.
243+
244+
Parameters that specify a specific type require that specific input type.
245+
246+
A relatively easy way to test your documentation is to build it on
247+
`readthedocs.org <https://readthedocs.org>` by following their
248+
`Import Your Docs <http://docs.readthedocs.io/en/stable/getting_started.html#import-your-docs>`_
249+
instructions and enabling your branch on the readthedocs
250+
`versions admin page <http://docs.readthedocs.io/en/stable/features.html#versions>`_.
251+
252+
Another option is to install the required dependencies in your virtual/conda
253+
environment. See
254+
`docs/environment.yml <https://github.com/pvlib/pvlib-python/blob/master/docs/environment.yml>`_
255+
for the latest dependences for building the complete documentation. Some
256+
doc files can be compiled with fewer dependencies, but this is beyond
257+
the scope of this guide.
258+
259+
.. _testing:
82260

83261
Testing
84262
~~~~~~~
85263

86-
pvlib's unit tests can easily be run by executing ``py.test`` on the
264+
pvlib's unit tests can easily be run by executing ``pytest`` on the
87265
pvlib directory:
88266

89267
``pytest pvlib``
@@ -96,7 +274,16 @@ or, for a single test:
96274

97275
``pytest pvlib/test/test_clearsky.py::test_ineichen_nans``
98276

99-
Use the ``--pdb`` flag to debug failures and avoid using ``print``.
277+
We suggest using pytest's ``--pdb`` flag to debug test failures rather
278+
than using ``print`` or ``logging`` calls. For example:
279+
280+
``pytest pvlib --pdb``
281+
282+
will drop you into the
283+
`pdb debugger <https://docs.python.org/3/library/pdb.html>`_ at the
284+
location of a test failure. As described in :ref:`code-style`, pvlib
285+
code does not use ``print`` or ``logging`` calls, and this also applies
286+
to the test suite (with rare exceptions).
100287

101288
New unit test code should be placed in the corresponding test module in
102289
the pvlib/test directory.
@@ -106,7 +293,7 @@ modifications to pvlib.
106293

107294
pvlib-python contains 3 "layers" of code: functions, PVSystem/Location,
108295
and ModelChain. Contributors will need to add tests that correspond to
109-
the layer that they modify.
296+
the layers that they modify.
110297

111298
Functions
112299
---------
@@ -148,6 +335,7 @@ in fact, happen. Finally, we check that the output of the method call is
148335
reasonable.
149336

150337
.. code-block:: python
338+
151339
def test_PVSystem_ashraeiam(mocker):
152340
# mocker is a pytest-mock object.
153341
# mocker.spy adds code to a function to keep track of how it is called
@@ -189,6 +377,7 @@ The example below shows how mock can be used to assert that the correct
189377
PVSystem method is called through ``ModelChain.run_model``.
190378

191379
.. code-block:: python
380+
192381
def test_modelchain_dc_model(mocker):
193382
# set up location and system for model chain
194383
location = location.Location(32, -111)
@@ -220,5 +409,5 @@ This documentation
220409

221410
If this documentation is unclear, help us improve it! Consider looking
222411
at the `pandas
223-
documentation <http://pandas.pydata.org/pandas-docs/version/0.18.1/
412+
documentation <http://pandas.pydata.org/pandas-docs/stable/
224413
contributing.html>`_ for inspiration.

docs/sphinx/source/package_overview.rst

+2-4
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,8 @@ change something about pvlib, then please make an issue on our
282282

283283
How do I contribute?
284284
--------------------
285-
We're so glad you asked! Please see our
286-
`wiki <https://github.com/pvlib/pvlib-python/wiki/Contributing-to-pvlib-python>`_
287-
for information and instructions on how to contribute.
288-
We really appreciate it!
285+
We're so glad you asked! Please see :ref:`Contributing` for information and
286+
instructions on how to contribute. We really appreciate it!
289287

290288

291289
Credits

docs/sphinx/source/whatsnew/v0.6.0.rst

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ Documentation
3434
objects, and ModelChain.
3535
* Updated several incorrect statements in ModelChain documentation regarding
3636
implementation status and default values. (:issue:`480`)
37+
* Expanded general contributing and pull request guidelines.
3738

3839

3940
Testing

0 commit comments

Comments
 (0)