Skip to content

Commit 4faecb4

Browse files
Clarifications in contributors guide (#7595)
* update links to issue labels * clarify that we actually use ruff now in pre-commit * mention absolufy-imports which we use in pre-commit * checked that example test file works * fix formatting of nested bullets * add pre-commit to docs env * clarify which environment to use for docstrings * explain how to view local html docs build * clarify what pip -e does * update git commit message requirements * clarify that small changes don't require local testing * whatsnew * point to CI build of the docs * add suggestion to use github desktop * image to help find view docs button on readthedocs * update outdated screenshot of green ci * internal link to point to where you can view RTD build * Add ruff as separate entry in list of pre-commit checks Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com> --------- Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
1 parent 485f801 commit 4faecb4

File tree

5 files changed

+128
-74
lines changed

5 files changed

+128
-74
lines changed

ci/requirements/doc.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ dependencies:
2323
- pandas>=1.4
2424
- pooch
2525
- pip
26+
- pre-commit
2627
- pyproj
2728
- rasterio>=1.1
2829
- scipy!=1.10.0

doc/_static/ci.png

-84.7 KB
Loading

doc/_static/view-docs.png

648 KB
Loading

doc/contributing.rst

Lines changed: 125 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ enhancements, and ideas are welcome.
1919
If you are brand new to *xarray* or open-source development, we recommend going
2020
through the `GitHub "issues" tab <https://github.com/pydata/xarray/issues>`_
2121
to find issues that interest you. There are a number of issues listed under
22-
`Documentation <https://github.com/pydata/xarray/issues?q=is%3Aissue+is%3Aopen+label%3Adocumentation>`_
22+
`Documentation <https://github.com/pydata/xarray/labels/topic-documentation>`_
2323
and `good first issue
24-
<https://github.com/pydata/xarray/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22>`_
24+
<https://github.com/pydata/xarray/labels/contrib-good-first-issue>`_
2525
where you could start out. Once you've found an interesting issue, you can
2626
return here to get your development environment setup.
2727

2828
Feel free to ask questions on the `mailing list
29-
<https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!forum/xarray>`_.
29+
<https://groups.google.com/g/xarray>`_.
3030

3131
.. _contributing.bug_reports:
3232

@@ -106,6 +106,12 @@ Getting started with Git
106106
setting up your SSH key, and configuring git. All these steps need to be completed before
107107
you can work seamlessly between your local repository and GitHub.
108108

109+
.. note::
110+
111+
The following instructions assume you want to learn how to interact with github via the git command-line utility,
112+
but contributors who are new to git may find it easier to use other tools instead such as
113+
`Github Desktop <https://desktop.github.com/>`_.
114+
109115
.. _contributing.forking:
110116

111117
Forking
@@ -122,17 +128,58 @@ want to clone your fork to your machine::
122128
This creates the directory `xarray` and connects your repository to
123129
the upstream (main project) *xarray* repository.
124130

131+
Creating a branch
132+
-----------------
133+
134+
You want your ``main`` branch to reflect only production-ready code, so create a
135+
feature branch before making your changes. For example::
136+
137+
git branch shiny-new-feature
138+
git checkout shiny-new-feature
139+
140+
The above can be simplified to::
141+
142+
git checkout -b shiny-new-feature
143+
144+
This changes your working directory to the shiny-new-feature branch. Keep any
145+
changes in this branch specific to one bug or feature so it is clear
146+
what the branch brings to *xarray*. You can have many "shiny-new-features"
147+
and switch in between them using the ``git checkout`` command.
148+
149+
To update this branch, you need to retrieve the changes from the ``main`` branch::
150+
151+
git fetch upstream
152+
git merge upstream/main
153+
154+
This will combine your commits with the latest *xarray* git ``main``. If this
155+
leads to merge conflicts, you must resolve these before submitting your pull
156+
request. If you have uncommitted changes, you will need to ``git stash`` them
157+
prior to updating. This will effectively store your changes, which can be
158+
reapplied after updating.
159+
125160
.. _contributing.dev_env:
126161

127162
Creating a development environment
128163
----------------------------------
129164

130-
To test out code changes, you'll need to build *xarray* from source, which
165+
To test out code changes locally, you'll need to build *xarray* from source, which
131166
requires a Python environment. If you're making documentation changes, you can
132167
skip to :ref:`contributing.documentation` but you won't be able to build the
133168
documentation locally before pushing your changes.
134169

135-
.. _contributiong.dev_python:
170+
.. note::
171+
172+
For small changes, such as fixing a typo, you don't necessarily need to build and test xarray locally.
173+
If you make your changes then :ref:`commit and push them to a new branch <contributing.changes>`,
174+
xarray's automated :ref:`continuous integration tests <contributing.ci>` will run and check your code in various ways.
175+
You can then try to fix these problems by committing and pushing more commits to the same branch.
176+
177+
You can also avoid building the documentation locally by instead :ref:`viewing the updated documentation via the CI <contributing.pr>`.
178+
179+
To speed up this feedback loop or for more complex development tasks you should build and test xarray locally.
180+
181+
182+
.. _contributing.dev_python:
136183

137184
Creating a Python Environment
138185
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -193,35 +240,6 @@ To return to your root environment::
193240

194241
See the full conda docs `here <http://conda.pydata.org/docs>`__.
195242

196-
Creating a branch
197-
-----------------
198-
199-
You want your ``main`` branch to reflect only production-ready code, so create a
200-
feature branch before making your changes. For example::
201-
202-
git branch shiny-new-feature
203-
git checkout shiny-new-feature
204-
205-
The above can be simplified to::
206-
207-
git checkout -b shiny-new-feature
208-
209-
This changes your working directory to the shiny-new-feature branch. Keep any
210-
changes in this branch specific to one bug or feature so it is clear
211-
what the branch brings to *xarray*. You can have many "shiny-new-features"
212-
and switch in between them using the ``git checkout`` command.
213-
214-
To update this branch, you need to retrieve the changes from the ``main`` branch::
215-
216-
git fetch upstream
217-
git merge upstream/main
218-
219-
This will combine your commits with the latest *xarray* git ``main``. If this
220-
leads to merge conflicts, you must resolve these before submitting your pull
221-
request. If you have uncommitted changes, you will need to ``git stash`` them
222-
prior to updating. This will effectively store your changes, which can be
223-
reapplied after updating.
224-
225243
.. _contributing.documentation:
226244

227245
Contributing to the documentation
@@ -302,6 +320,7 @@ Requirements
302320
~~~~~~~~~~~~
303321
Make sure to follow the instructions on :ref:`creating a development environment above <contributing.dev_env>`, but
304322
to build the docs you need to use the environment file ``ci/requirements/doc.yml``.
323+
You should also use this environment and these steps if you want to view changes you've made to the docstrings.
305324

306325
.. code-block:: sh
307326
@@ -312,7 +331,7 @@ to build the docs you need to use the environment file ``ci/requirements/doc.yml
312331
# or with older versions of Anaconda:
313332
source activate xarray-docs
314333
315-
# Build and install xarray
334+
# Build and install a local, editable version of xarray
316335
pip install -e .
317336
318337
Building the documentation
@@ -323,7 +342,17 @@ To build the documentation run::
323342
cd doc/
324343
make html
325344

326-
Then you can find the HTML output in the folder ``xarray/doc/_build/html/``.
345+
Then you can find the HTML output files in the folder ``xarray/doc/_build/html/``.
346+
347+
To see what the documentation now looks like with your changes, you can view the HTML build locally by opening the files in your local browser.
348+
For example, if you normally use Google Chrome as your browser, you could enter::
349+
350+
google-chrome _build/html/quick-overview.html
351+
352+
in the terminal, running from within the ``doc/`` folder.
353+
You should now see a new tab pop open in your local browser showing the ``quick-overview`` page of the documentation.
354+
The different pages of this local build of the documentation are linked together,
355+
so you can browse the whole documentation by following links the same way you would on the officially-hosted xarray docs site.
327356

328357
The first time you build the docs, it will take quite a while because it has to run
329358
all the code examples and build all the generated docstring pages. In subsequent
@@ -362,14 +391,13 @@ Code Formatting
362391
xarray uses several tools to ensure a consistent code format throughout the project:
363392

364393
- `Black <https://black.readthedocs.io/en/stable/>`_ for standardized
365-
code formatting
394+
code formatting,
366395
- `blackdoc <https://blackdoc.readthedocs.io/en/stable/>`_ for
367-
standardized code formatting in documentation
368-
- `Flake8 <http://flake8.pycqa.org/en/latest/>`_ for general code quality
369-
- `isort <https://github.com/timothycrosley/isort>`_ for standardized order in imports.
370-
See also `flake8-isort <https://github.com/gforcada/flake8-isort>`_.
396+
standardized code formatting in documentation,
397+
- `ruff <https://github.com/charliermarsh/ruff/>`_ for code quality checks and standardized order in imports
398+
- `absolufy-imports <https://github.com/MarcoGorelli/absolufy-imports>`_ for absolute instead of relative imports from different files,
371399
- `mypy <http://mypy-lang.org/>`_ for static type checking on `type hints
372-
<https://docs.python.org/3/library/typing.html>`_
400+
<https://docs.python.org/3/library/typing.html>`_.
373401

374402
We highly recommend that you setup `pre-commit hooks <https://pre-commit.com/>`_
375403
to automatically run all the above tools every time you make a git commit. This
@@ -418,7 +446,7 @@ of xarray, and for developers of other libraries that depend on xarray.
418446
Testing With Continuous Integration
419447
-----------------------------------
420448

421-
The *xarray* test suite runs automatically the
449+
The *xarray* test suite runs automatically via the
422450
`GitHub Actions <https://docs.github.com/en/free-pro-team@latest/actions>`__,
423451
continuous integration service, once your pull request is submitted.
424452

@@ -480,7 +508,7 @@ the expected correct result::
480508
Transitioning to ``pytest``
481509
~~~~~~~~~~~~~~~~~~~~~~~~~~~
482510

483-
*xarray* existing test structure is *mostly* classed based, meaning that you will
511+
*xarray* existing test structure is *mostly* class-based, meaning that you will
484512
typically find tests wrapped in a class.
485513

486514
.. code-block:: python
@@ -517,8 +545,6 @@ features that we like to use.
517545
We would name this file ``test_cool_feature.py`` and put in an appropriate place in the
518546
``xarray/tests/`` structure.
519547

520-
.. TODO: confirm that this actually works
521-
522548
.. code-block:: python
523549
524550
import pytest
@@ -569,26 +595,27 @@ A test run of this yields
569595

570596
.. code-block:: shell
571597
572-
((xarray) $ pytest test_cool_feature.py -v
573-
=============================== test session starts ================================
598+
((xarray) $ pytest test_cool_feature.py -v
599+
================================= test session starts ==================================
574600
platform darwin -- Python 3.10.6, pytest-7.2.0, pluggy-1.0.0 --
575601
cachedir: .pytest_cache
576602
plugins: hypothesis-6.56.3, cov-4.0.0
577603
collected 11 items
578604
579-
test_cool_feature.py::test_dtypes[int8] PASSED
580-
test_cool_feature.py::test_dtypes[int16] PASSED
581-
test_cool_feature.py::test_dtypes[int32] PASSED
582-
test_cool_feature.py::test_dtypes[int64] PASSED
583-
test_cool_feature.py::test_mark[float32] PASSED
584-
test_cool_feature.py::test_mark[int16] SKIPPED
585-
test_cool_feature.py::test_mark[int32] xfail
586-
test_cool_feature.py::test_series[int8] PASSED
587-
test_cool_feature.py::test_series[int16] PASSED
588-
test_cool_feature.py::test_series[int32] PASSED
589-
test_cool_feature.py::test_series[int64] PASSED
605+
xarray/tests/test_cool_feature.py::test_dtypes[int8] PASSED [ 9%]
606+
xarray/tests/test_cool_feature.py::test_dtypes[int16] PASSED [ 18%]
607+
xarray/tests/test_cool_feature.py::test_dtypes[int32] PASSED [ 27%]
608+
xarray/tests/test_cool_feature.py::test_dtypes[int64] PASSED [ 36%]
609+
xarray/tests/test_cool_feature.py::test_mark[float32] PASSED [ 45%]
610+
xarray/tests/test_cool_feature.py::test_mark[int16] SKIPPED (unconditional skip) [ 54%]
611+
xarray/tests/test_cool_feature.py::test_mark[int32] XFAIL (to show how it works) [ 63%]
612+
xarray/tests/test_cool_feature.py::test_series[int8] PASSED [ 72%]
613+
xarray/tests/test_cool_feature.py::test_series[int16] PASSED [ 81%]
614+
xarray/tests/test_cool_feature.py::test_series[int32] PASSED [ 90%]
615+
xarray/tests/test_cool_feature.py::test_series[int64] PASSED [100%]
616+
590617
591-
================== 9 passed, 1 skipped, 1 xfailed in 1.83 seconds ==================
618+
==================== 9 passed, 1 skipped, 1 xfailed in 1.83 seconds ====================
592619
593620
Tests that we have ``parametrized`` are now accessible via the test name, for
594621
example we could run these with ``-k int8`` to sub-select *only* those tests
@@ -598,10 +625,10 @@ which match ``int8``.
598625
.. code-block:: shell
599626
600627
((xarray) bash-3.2$ pytest test_cool_feature.py -v -k int8
601-
=========================== test session starts ===========================
602-
platform darwin -- Python 3.10.6, pytest-7.2.0, pluggy-1.0.0 --
603-
cachedir: .pytest_cache
604-
plugins: hypothesis-6.56.3, cov-4.0.0
628+
================================== test session starts ==================================
629+
platform darwin -- Python 3.10.6, pytest-7.2.0, pluggy-1.0.0 --
630+
cachedir: .pytest_cache
631+
plugins: hypothesis-6.56.3, cov-4.0.0
605632
collected 11 items
606633
607634
test_cool_feature.py::test_dtypes[int8] PASSED
@@ -728,9 +755,13 @@ If your code is an enhancement, it is most likely necessary to add usage
728755
examples to the existing documentation. This can be done following the section
729756
regarding documentation :ref:`above <contributing.documentation>`.
730757
758+
.. _contributing.changes:
759+
731760
Contributing your changes to *xarray*
732761
=====================================
733762
763+
.. _contributing.committing:
764+
734765
Committing your code
735766
--------------------
736767
@@ -751,11 +782,11 @@ Doing 'git status' again should give something like::
751782
# modified: /relative/path/to/file-you-added.py
752783
#
753784
754-
The following defines how a commit message should be structured:
785+
The following defines how a commit message should ideally be structured:
755786
756-
* A subject line with `< 72` chars.
757-
* One blank line.
758-
* Optionally, a commit message body.
787+
* A subject line with `< 72` chars.
788+
* One blank line.
789+
* Optionally, a commit message body.
759790
760791
Please reference the relevant GitHub issues in your commit message using ``GH1234`` or
761792
``#1234``. Either style is fine, but the former is generally preferred.
@@ -764,6 +795,9 @@ Now you can commit your changes in your local repository::
764795
765796
git commit -m
766797
798+
799+
.. _contributing.pushing:
800+
767801
Pushing your changes
768802
--------------------
769803
@@ -788,6 +822,8 @@ like::
788822
Now your code is on GitHub, but it is not yet a part of the *xarray* project. For that to
789823
happen, a pull request needs to be submitted on GitHub.
790824
825+
.. _contributing.review:
826+
791827
Review your code
792828
----------------
793829
@@ -802,6 +838,8 @@ double check your branch changes against the branch it was based on:
802838
#. Select the ``base`` and ``compare`` branches, if necessary. This will be ``main`` and
803839
``shiny-new-feature``, respectively.
804840
841+
.. _contributing.pr:
842+
805843
Finally, make the pull request
806844
------------------------------
807845
@@ -819,7 +857,16 @@ release. To submit a pull request:
819857
#. Click ``Send Pull Request``.
820858
821859
This request then goes to the repository maintainers, and they will review
822-
the code. If you need to make more changes, you can make them in
860+
the code.
861+
862+
If you have made updates to the documentation, you can now see a preview of the updated docs by clicking on "Details" under
863+
the ``docs/readthedocs.org`` check near the bottom of the list of checks that run automatically when submitting a PR,
864+
then clicking on the "View Docs" button on the right (not the big green button, the small black one further down).
865+
866+
.. image:: _static/view-docs.png
867+
868+
869+
If you need to make more changes, you can make them in
823870
your branch, add them to a new commit, push them to GitHub, and the pull request
824871
will automatically be updated. Pushing them to GitHub again is done by::
825872
@@ -829,6 +876,8 @@ This will automatically update your pull request with the latest code and restar
829876
:ref:`Continuous Integration <contributing.ci>` tests.
830877
831878
879+
.. _contributing.delete:
880+
832881
Delete your merged branch (optional)
833882
------------------------------------
834883
@@ -853,20 +902,22 @@ GitHub. To delete it there do::
853902
git push origin --delete shiny-new-feature
854903
855904
905+
.. _contributing.checklist:
906+
856907
PR checklist
857908
------------
858909
859910
- **Properly comment and document your code.** See `"Documenting your code" <https://docs.xarray.dev/en/stable/contributing.html#documenting-your-code>`_.
860911
- **Test that the documentation builds correctly** by typing ``make html`` in the ``doc`` directory. This is not strictly necessary, but this may be easier than waiting for CI to catch a mistake. See `"Contributing to the documentation" <https://docs.xarray.dev/en/stable/contributing.html#contributing-to-the-documentation>`_.
861912
- **Test your code**.
862913
863-
- Write new tests if needed. See `"Test-driven development/code writing" <https://docs.xarray.dev/en/stable/contributing.html#test-driven-development-code-writing>`_.
864-
- Test the code using `Pytest <http://doc.pytest.org/en/latest/>`_. Running all tests (type ``pytest`` in the root directory) takes a while, so feel free to only run the tests you think are needed based on your PR (example: ``pytest xarray/tests/test_dataarray.py``). CI will catch any failing tests.
865-
- By default, the upstream dev CI is disabled on pull request and push events. You can override this behavior per commit by adding a <tt>[test-upstream]</tt> tag to the first line of the commit message. For documentation-only commits, you can skip the CI per commit by adding a "[skip-ci]" tag to the first line of the commit message.
914+
- Write new tests if needed. See `"Test-driven development/code writing" <https://docs.xarray.dev/en/stable/contributing.html#test-driven-development-code-writing>`_.
915+
- Test the code using `Pytest <http://doc.pytest.org/en/latest/>`_. Running all tests (type ``pytest`` in the root directory) takes a while, so feel free to only run the tests you think are needed based on your PR (example: ``pytest xarray/tests/test_dataarray.py``). CI will catch any failing tests.
916+
- By default, the upstream dev CI is disabled on pull request and push events. You can override this behavior per commit by adding a <tt>[test-upstream]</tt> tag to the first line of the commit message. For documentation-only commits, you can skip the CI per commit by adding a "[skip-ci]" tag to the first line of the commit message.
866917
867918
- **Properly format your code** and verify that it passes the formatting guidelines set by `Black <https://black.readthedocs.io/en/stable/>`_ and `Flake8 <http://flake8.pycqa.org/en/latest/>`_. See `"Code formatting" <https://docs.xarray.dev/en/stablcontributing.html#code-formatting>`_. You can use `pre-commit <https://pre-commit.com/>`_ to run these automatically on each commit.
868919
869-
- Run ``pre-commit run --all-files`` in the root directory. This may modify some files. Confirm and commit any formatting changes.
920+
- Run ``pre-commit run --all-files`` in the root directory. This may modify some files. Confirm and commit any formatting changes.
870921
871-
- **Push your code and** `create a PR on GitHub <https://help.github.com/en/articles/creating-a-pull-request>`_.
922+
- **Push your code** and `create a PR on GitHub <https://help.github.com/en/articles/creating-a-pull-request>`_.
872923
- **Use a helpful title for your pull request** by summarizing the main contributions rather than using the latest commit message. If the PR addresses an `issue <https://github.com/pydata/xarray/issues>`_, please `reference it <https://help.github.com/en/articles/autolinked-references-and-urls>`_.

doc/whats-new.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ Bug fixes
6060
Documentation
6161
~~~~~~~~~~~~~
6262

63+
- Clarify language in contributor's guide (:issue:`7495`, :pull:`7595`)
64+
By `Tom Nicholas <https://github.com/TomNicholas>`_.
6365

6466
Internal Changes
6567
~~~~~~~~~~~~~~~~

0 commit comments

Comments
 (0)