Merge pull request #1018 from effigies/doc/cleanup

matthew-brett · web-flow · commit 788d6c71faa9 · 2021-05-14T14:09:37.000+01:00
DOC: Cleanups to roadmap, governance, BIAPs
diff --git a/doc/source/devel/biaps/biap_0000.rst b/doc/source/devel/biaps/biap_0000.rst
@@ -1,7 +1,7 @@
 .. _biap0:
 
 ============================
-BIAP 0 — Purpose and Process
+BIAP 0 - Purpose and process
 ============================
 
 :Author: Jarrod Millman <millman@berkeley.edu>
diff --git a/doc/source/devel/biaps/biap_0001.rst b/doc/source/devel/biaps/biap_0001.rst
@@ -1,5 +1,7 @@
+.. _biap1:
+
 ################################
-BIAP1 - towards immutable images
+BIAP1 - Towards immutable images
 ################################
 
 :Author: Matthew Brett
diff --git a/doc/source/devel/biaps/biap_0002.rst b/doc/source/devel/biaps/biap_0002.rst
@@ -1,5 +1,7 @@
+.. _biap2:
+
 #################
-BIAP2 : Slicecopy
+BIAP2 - Slicecopy
 #################
 
 :Author: Matthew Brett
diff --git a/doc/source/devel/biaps/biap_0003.rst b/doc/source/devel/biaps/biap_0003.rst
@@ -1,5 +1,7 @@
+.. _biap3:
+
 #####################################
-BIAP3 - a JSON nifti header extension
+BIAP3 - A JSON nifti header extension
 #####################################
 
 :Author: Matthew Brett, Bob Dougherty
diff --git a/doc/source/devel/biaps/biap_0004.rst b/doc/source/devel/biaps/biap_0004.rst
@@ -1,5 +1,7 @@
+.. _biap4:
+
 ####################################
-BIAP4 - merging nibabel and dcmstack
+BIAP4 - Merging nibabel and dcmstack
 ####################################
 
 :Author: Brendan Moloney, Matthew Brett
diff --git a/doc/source/devel/biaps/biap_0005.rst b/doc/source/devel/biaps/biap_0005.rst
@@ -1,5 +1,7 @@
+.. _biap5:
+
 ###############################
-BIAP5 - a streamlines converter
+BIAP5 - A streamlines converter
 ###############################
 
 :Author: Marc-Alexandre Côté
diff --git a/doc/source/devel/biaps/biap_0006.rst b/doc/source/devel/biaps/biap_0006.rst
@@ -1,5 +1,7 @@
+.. _biap6:
+
 ##############################
-BIAP6 - identifying image axes
+BIAP6 - Identifying image axes
 ##############################
 
 :Author: Matthew Brett
diff --git a/doc/source/devel/biaps/biap_0007.rst b/doc/source/devel/biaps/biap_0007.rst
@@ -1,5 +1,7 @@
+.. _biap7:
+
 ###############################
-BIAP7 - loading multiple images
+BIAP7 - Loading multiple images
 ###############################
 
 :Author: Matthew Brett
diff --git a/doc/source/devel/biaps/biap_0008.rst b/doc/source/devel/biaps/biap_0008.rst
@@ -1,5 +1,7 @@
+.. _biap8:
+
 ################################################
-BIAP8 - always load image data as floating point
+BIAP8 - Always load image data as floating point
 ################################################
 
 :Author: Matthew Brett
diff --git a/doc/source/devel/biaps/biap_template.rst b/doc/source/devel/biaps/biap_template.rst
@@ -1,3 +1,5 @@
+.. _biap_template:
+
 ==================================
 BIAP X — Template and Instructions
 ==================================
diff --git a/doc/source/devel/core_developer.rst b/doc/source/devel/core_developer.rst
@@ -0,0 +1,152 @@
+.. _core_dev:
+
+Core Developer Guide
+====================
+
+As a core developer, you should continue making pull requests
+in accordance with the :ref:`chap_devguide`.
+You are responsible for shepherding other contributors through the review process.
+You also have the ability to merge or approve other contributors' pull requests.
+
+Reviewing
+---------
+
+How to Conduct A Good Review
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+*Always* be kind to contributors. Nearly all of Nibabel is
+volunteer work, for which we are tremendously grateful. Provide
+constructive criticism on ideas and implementations, and remind
+yourself of how it felt when your own work was being evaluated as a
+novice.
+
+Nibabel strongly values mentorship in code review.  New users
+often need more handholding, having little to no git
+experience. Repeat yourself liberally, and, if you don’t recognize a
+contributor, point them to our development guide, or other GitHub
+workflow tutorials around the web. Do not assume that they know how
+GitHub works (e.g., many don't realize that adding a commit
+automatically updates a pull request). Gentle, polite, kind
+encouragement can make the difference between a new core developer and
+an abandoned pull request.
+
+When reviewing, focus on the following:
+
+1. **API:** The API is what users see when they first use
+   Nibabel. APIs are difficult to change once released, so
+   should be simple, consistent with other parts of the library, and
+   should avoid side-effects such as changing global state or modifying
+   input variables.
+
+2. **Documentation:** Any new feature should have a tutorial
+   example that not only illustrates but explains it.
+
+3. **The algorithm:** You should understand the code being modified or
+   added before approving it.  (See `Merge Only Changes You
+   Understand`_ below.) Implementations should do what they claim,
+   and be simple, readable, and efficient.
+
+4. **Tests:** All contributions to the library *must* be tested, and
+   each added line of code should be covered by at least one test. Good
+   tests not only execute the code, but explore corner cases.  It is tempting
+   not to review tests, but please do so.
+
+Other changes may be *nitpicky*: spelling mistakes, formatting,
+etc. Do not ask contributors to make these changes, and instead
+make the changes by `pushing to their branch
+<https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork>`__,
+or using GitHub’s `suggestion
+<https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/commenting-on-a-pull-request>`__
+`feature
+<https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request>`__.
+(The latter is preferred because it gives the contributor a choice in
+whether to accept the changes.)
+
+Please add a note to a pull request after you push new changes; GitHub
+may not send out notifications for these.
+
+Merge Only Changes You Understand
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+*Long-term maintainability* is an important concern.  Code doesn't
+merely have to *work*, but should be *understood* by multiple core
+developers.  Changes will have to be made in the future, and the
+original contributor may have moved on.
+
+Therefore, *do not merge a code change unless you understand it*. Ask
+for help freely: we have a long history of consulting community
+members, or even external developers, for added insight where needed,
+and see this as a great learning opportunity.
+
+While we collectively "own" any patches (and bugs!) that become part
+of the code base, you are vouching for changes you merge.  Please take
+that responsibility seriously.
+
+Closing issues and pull requests
+--------------------------------
+
+Sometimes, an issue must be closed that was not fully resolved. This can be
+for a number of reasons:
+
+- the person behind the original post has not responded to calls for
+  clarification, and none of the core developers have been able to reproduce
+  their issue;
+- fixing the issue is difficult, and it is deemed too niche a use case to
+  devote sustained effort or prioritize over other issues; or
+- the use case or feature request is something that core developers feel
+  does not belong in Nibabel,
+
+among others. Similarly, pull requests sometimes need to be closed without
+merging, because:
+
+- the pull request implements a niche feature that we consider not worth the
+  added maintenance burden;
+- the pull request implements a useful feature, but requires significant
+  effort to bring up to Nibabel's standards, and the original
+  contributor has moved on, and no other developer can be found to make the
+  necessary changes; or
+- the pull request makes changes that make maintenance harder, such as
+  increasing the code complexity of a function significantly to implement a
+  marginal speedup,
+
+among others.
+
+All these may be valid reasons for closing, but we must be wary not to alienate
+contributors by closing an issue or pull request without an explanation. When
+closing, your message should:
+
+- explain clearly how the decision was made to close. This is particularly
+  important when the decision was made in a community meeting, which does not
+  have as visible a record as the comments thread on the issue itself;
+- thank the contributor(s) for their work; and
+- provide a clear path for the contributor or anyone else to appeal the
+  decision.
+
+These points help ensure that all contributors feel welcome and empowered to
+keep contributing, regardless of the outcome of past contributions.
+
+Further resources
+-----------------
+
+As a core member, you should be familiar with community and developer
+resources such as:
+
+-  Our :ref:`chap_devguide`
+-  Our :ref:`community_guidelines`
+-  `PEP8 <https://www.python.org/dev/peps/pep-0008/>`__ for Python style
+-  `PEP257 <https://www.python.org/dev/peps/pep-0257/>`__ and the `NumPy
+   documentation
+   guide <https://numpy.org/doc/stable/docs/howto_document.html>`__
+   for docstrings. (NumPy docstrings are a superset of PEP257. You
+   should read both.)
+-  The Nibabel `tag on NeuroStars <https://neurostars.org/tag/nibabel>`__
+-  Our `mailing
+   list <https://mail.python.org/mailman/listinfo/neuroimaging>`__
+
+Please do monitor any of the resources above that you find helpful.
+
+Acknowledgments
+===============
+
+This document is based on the `NetworkX Core Developer guide
+<https://networkx.org/documentation/latest/developer/core_developer.html>`_.
diff --git a/doc/source/devel/governance.rst b/doc/source/devel/governance.rst
@@ -40,7 +40,7 @@ project in concrete ways, such as:
 among other possibilities. By contributing to the project, community members
 can directly help to shape its future.
 
-Contributors should read the :ref:`contributor_guide` and our :ref:`code_of_conduct`.
+Contributors should read the :ref:`chap_devguide` and our :ref:`community_guidelines`.
 
 Core Developers
 ---------------
@@ -83,7 +83,7 @@ in a reasonable timeframe, the SC is the entity that resolves the issue.
 Steering Council members appear as team members on the `Nibabel Steering
 Council Team page
 <https://github.com/orgs/nipy/teams/nibabel-steering-council/members>`_ and
-can be messaged ``@nipy/nibabel-steering-council``. Core
+can be messaged ``@nipy/nibabel-steering-council``.
 
 Decision Making Process
 =======================
@@ -139,7 +139,7 @@ Enhancement Proposals (BIAPs)
 =============================
 
 Any proposals for enhancements of Nibabel should be written as a formal BIAP
-following the template :doc:`biap_template`. The BIAP must be made public and
+following the template :ref:`biap_template`. The BIAP must be made public and
 discussed before any vote is taken. The discussion must be summarized by a key
 advocate of the proposal in the appropriate section of the BIAP. Once this
 summary is made public and after sufficient time to allow the core team to
diff --git a/doc/source/devel/index.rst b/doc/source/devel/index.rst
@@ -10,8 +10,10 @@ Developer documentation page
     :maxdepth: 2
 
     devguide
+    core_developer
     governance
     roadmap
+    biaps/index
     add_test_data
     add_image_format
     devdiscuss
diff --git a/doc/source/devel/roadmap.rst b/doc/source/devel/roadmap.rst
@@ -8,7 +8,7 @@ continue to be tracked on our issue tracker.
 
 The scope of these improvements means that these changes may be controversial,
 are likely to involve significant discussion among the core development team,
-and may require the creation of one or more NIAPs (Nibabel Increased
+and may require the creation of one or more BIAPs (niBabel Increased
 Awesomeness Proposals).
 
 ==========
@@ -37,24 +37,24 @@ way the image was acquired.  Axes have natural labels, expressing meaning,
 such as "time" or "slice", and they may have tick labels such as acquisition
 time. The scanner captures this information, but typical image formats cannot
 store it, so it is easy to lose metadata and make analysis errors; see
-:doc:`biap_0006`.
+:ref:`biap6`.
 
 We plan to expand Nibabel's API to encode axis and tick labels by integrating
-the Xarray package: `http://xarray.pydata.org`_.  Xarray simplifies HDF5
-serialization, and visualization; see `https://napari.org`_.
+the `Xarray package <http://xarray.pydata.org>`_.  Xarray simplifies HDF5
+serialization, and visualization.
 
 An API for labels is not useful if we cannot read labels from the scanner
 data, or save them with the image.  We plan to:
 
 * Develop HDF5 equivalents of standard image formats, for serialization of
   data with labels.
 * Expand the current standard image format, NIfTI, to store labels in a JSON
-  addition to image metadata: `https://github.com/nipy/nibabel/wiki/BIAP3`_.
+  addition to image metadata: :ref:`biap3`.
 * Read image metadata from DICOM, the standard scanner format.
 
-Reading and attaching DICOM data will start with code integrated from Dcmstack,
-by Brendan Moloney: `https://github.com/moloney/dcmstack`_; see:
-`https://github.com/nipy/nibabel/wiki/BIAP4`_.
+Reading and attaching DICOM data will start with code integrated from
+`Dcmstack <https://github.com/moloney/dcmstack>`_, by Brendan Moloney; see:
+:ref:`biap4`.
 
 DICOM metadata is often hidden inside "private" DICOM elements that need
 specialized parsers. We want to expand these parsers to preserve full metadata
@@ -67,10 +67,12 @@ API for surface data
 Neuroimaging data often refers to locations on the brain surface.  There are
 three common formats for such data: GIFTI, CIFTI and Freesurfer.  Nibabel can
 read these formats, but lacks a standard API for reading and storing surface
-data with metadata; see `https://github.com/nipy/nibabel/issues/936`_,
-`https://github.com/nilearn/nilearn/issues/2171`_.  We plan to develop
-a standard API, apply it to the standard formats, and design an efficient
-general HDF5 storage container for serializing surface data and metadata.
+data with metadata; see
+`nipy/nibabel#936 <https://github.com/nipy/nibabel/issues/936>`_,
+`nilearn/nilearn#2171 <https://github.com/nilearn/nilearn/issues/2171>`_.
+We plan to develop a standard API, apply it to the standard formats,
+and design an efficient general HDF5 storage container for serializing surface
+data and metadata.
 
 Range
 =====

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+.. _biap_template:`
	`2`	`+`
`1`	`3`	`==================================`
`2`	`4`	`BIAP X — Template and Instructions`
`3`	`5`	`==================================`