summaryrefslogtreecommitdiff
path: root/CONTRIBUTING.rst
diff options
context:
space:
mode:
authorJarrod Millman <jarrod.millman@gmail.com>2020-07-23 19:41:01 -0700
committerGitHub <noreply@github.com>2020-07-23 19:41:01 -0700
commit0f652331a4affc8912c80d84611a71e5a7921983 (patch)
tree4a577c7f31a016883a0b9b69549c7213e5ded4aa /CONTRIBUTING.rst
parent974baf9e7d829a218f49bc246d8c7e1dd6f0e155 (diff)
downloadnetworkx-0f652331a4affc8912c80d84611a71e5a7921983.tar.gz
Update contributor guide (#4088)
Diffstat (limited to 'CONTRIBUTING.rst')
-rw-r--r--CONTRIBUTING.rst320
1 files changed, 142 insertions, 178 deletions
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index db212c1f..7b40d63c 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -3,6 +3,9 @@
Contributor Guide
=================
+Development Workflow
+--------------------
+
1. If you are a first-time contributor:
* Go to `https://github.com/networkx/networkx
@@ -19,8 +22,59 @@ Contributor Guide
* Now, you have remote repositories named:
- - ``upstream``, which refers to the ``networkx`` repository
- - ``origin``, which refers to your personal fork
+ - ``upstream``, which refers to the ``networkx`` repository
+ - ``origin``, which refers to your personal fork
+
+ * Next, you need to set up your build environment.
+ Here are instructions for two popular environment managers:
+
+ * ``venv`` (pip based)
+
+ ::
+
+ # Create a virtualenv named ``networkx-dev`` that lives in the directory of
+ # the same name
+ python -m venv networkx-dev
+ # Activate it
+ source networkx-dev/bin/activate
+ # Install main development and runtime dependencies of networkx
+ pip install -r <(cat requirements/{default,developer,doc,optional,test}.txt)
+ #
+ # (Optional) Install pygraphviz, pydot, and gdal packages
+ # These packages require that you have your system properly configured
+ # and what that involves differs on various systems.
+ # pip install -r requirements/extras.txt
+ #
+ # Build and install networkx from source
+ pip install -e .
+ # Test your installation
+ PYTHONPATH=. pytest networkx
+
+ * ``conda`` (Anaconda or Miniconda)
+
+ ::
+
+ # Create a conda environment named ``networkx-dev``
+ conda create --name networkx-dev
+ # Activate it
+ conda activate networkx-dev
+ # Install main development and runtime dependencies of networkx
+ conda install -c conda-forge `for i in requirements/{default,developer,doc,optional,test}.txt; do echo -n " --file $i "; done`
+ #
+ # (Optional) Install pygraphviz, pydot, and gdal packages
+ # These packages require that you have your system properly configured
+ # and what that involves differs on various systems.
+ # pip install -r requirements/extras.txt
+ #
+ # Install networkx from source
+ pip install -e . --no-deps
+ # Test your installation
+ PYTHONPATH=. pytest networkx
+
+ * Finally, we recommend you use a pre-commit hook, which runs black when
+ you type ``git commit``::
+
+ pre-commit install
2. Develop your contribution:
@@ -45,14 +99,10 @@ Contributor Guide
* Running the tests locally *before* submitting a pull request helps catch
problems early and reduces the load on the continuous integration
- system. To ensure you have a properly-configured development environment
- for running the tests, see `Build environment setup`_.
-
-4. Format your code:
+ system.
- * We use psf/black to format Python code.
-5. To submit your contribution:
+4. Submit your contribution:
* Push your changes back to your fork on GitHub::
@@ -69,37 +119,40 @@ For a more detailed discussion, read these :doc:`detailed documents
<gitwash/index>` on how to use Git with ``networkx``
(`<https://networkx.github.io/documentation/latest/developer/gitwash/index.html>`_).
-6. Review process:
+5. Review process:
- * Reviewers (the other developers and interested community members) will
- write inline and/or general comments on your Pull Request (PR) to help
- you improve its implementation, documentation, and style. Every single
- developer working on the project has their code reviewed, and we've come
- to see it as friendly conversation from which we all learn and the
- overall code quality benefits. Therefore, please don't let the review
- discourage you from contributing: its only aim is to improve the quality
- of project, not to criticize (we are, after all, very grateful for the
- time you're donating!).
+ * Reviewers (the other developers and interested community members) will
+ write inline and/or general comments on your Pull Request (PR) to help
+ you improve its implementation, documentation, and style. Every single
+ developer working on the project has their code reviewed, and we've come
+ to see it as friendly conversation from which we all learn and the
+ overall code quality benefits. Therefore, please don't let the review
+ discourage you from contributing: its only aim is to improve the quality
+ of project, not to criticize (we are, after all, very grateful for the
+ time you're donating!).
- * To update your pull request, make your changes on your local repository
- and commit. As soon as those changes are pushed up (to the same branch as
- before) the pull request will update automatically.
+ * To update your pull request, make your changes on your local repository
+ and commit. As soon as those changes are pushed up (to the same branch as
+ before) the pull request will update automatically.
- * `Travis-CI <https://travis-ci.org/>`_, a continuous integration service,
- is triggered after each Pull Request update to build the code and run unit
- tests of your branch. The Travis tests must pass before your PR can be merged.
- If Travis fails, you can find out why by clicking on the "failed" icon (red
- cross) and inspecting the build and test log.
+ * `Travis-CI <https://travis-ci.org/>`_, a continuous integration service,
+ is triggered after each Pull Request update to build the code and run unit
+ tests of your branch. The Travis tests must pass before your PR can be merged.
+ If Travis fails, you can find out why by clicking on the "failed" icon (red
+ cross) and inspecting the build and test log.
- * `AppVeyor <http://ci.appveyor.com>`_, is another continuous integration
- service, which we use. You will also need to make sure that the AppVeyor
- tests pass.
+ * `AppVeyor <http://ci.appveyor.com>`_, is another continuous integration
+ service that we use. You will also need to make sure that the AppVeyor
+ tests pass.
-.. note::
+ .. note::
- If closing a bug, also add "Fixes #1480" where 1480 is the issue number.
+ If the PR closes an issue, make sure that GitHub knows to automatically
+ close the issue when the PR is merged. For example, if the PR closes
+ issue number 1480, you could use the phrase "Fixes #1480" in the PR
+ description or commit message.
-7. Document changes
+6. Document changes
If your change introduces any API modifications, please update
``doc/release/release_dev.rst``.
@@ -108,84 +161,61 @@ For a more detailed discussion, read these :doc:`detailed documents
``doc/developer/deprecations.rst`` for the team to remove the
deprecated functionality in the future.
-.. note::
+ .. note::
+
+ To reviewers: make sure the merge message has a brief description of the
+ change(s) and if the PR closes an issue add, for example, "Closes #123"
+ where 123 is the issue number.
- To reviewers: make sure the merge message has a brief description of the
- change(s) and if the PR closes an issue add, for example, "Closes #123"
- where 123 is the issue number.
+Divergence from ``upstream master``
+-----------------------------------
-Divergence between ``upstream master`` and your feature branch
---------------------------------------------------------------
+If GitHub indicates that the branch of your Pull Request can no longer
+be merged automatically, merge the master branch into yours::
-Never merge the main branch into yours. If GitHub indicates that the
-branch of your Pull Request can no longer be merged automatically, rebase
-onto master::
+ git fetch upstream master
+ git merge upstream/master
- git checkout master
- git pull upstream master
- git checkout bugfix-for-issue-1480
- git rebase master
+If any conflicts occur, they need to be fixed before continuing. See
+which files are in conflict using::
-If any conflicts occur, fix the according files and continue::
+ git status
- git add conflict-file1 conflict-file2
- git rebase --continue
+Which displays a message like::
-However, you should only rebase your own branches and must generally not
-rebase any branch which you collaborate on with someone else.
+ Unmerged paths:
+ (use "git add <file>..." to mark resolution)
-Finally, you must push your rebased branch::
+ both modified: file_with_conflict.txt
- git push --force origin bugfix-for-issue-1480
+Inside the conflicted file, you'll find sections like these::
-(If you are curious, here's a further discussion on the
-`dangers of rebasing <http://tinyurl.com/lll385>`_.
-Also see this `LWN article <http://tinyurl.com/nqcbkj>`_.)
+ <<<<<<< HEAD
+ The way the text looks in your branch
+ =======
+ The way the text looks in the master branch
+ >>>>>>> master
-Build environment setup
------------------------
+Choose one version of the text that should be kept, and delete the
+rest::
-Once you've cloned your fork of the networkx repository,
-you should set up a Python development environment tailored for networkx.
-You may choose the environment manager of your choice.
-Here we provide instructions for two popular environment managers:
-``venv`` (pip based) and ``conda`` (Anaconda or Miniconda).
+ The way the text looks in your branch
-venv
-^^^^
-When using ``venv``, you may find the following bash commands useful::
+Now, add the fixed file::
- # Create a virtualenv named ``networkx-dev`` that lives in the directory of
- # the same name
- python -m venv networkx-dev
- # Activate it
- source networkx-dev/bin/activate
- # Install all development and runtime dependencies of networkx
- pip install -r <(cat requirements/*.txt)
- # Build and install networkx from source
- pip install -e .
- # Test your installation
- PYTHONPATH=. pytest networkx
-conda
-^^^^^
+ git add file_with_conflict.txt
-When using conda, you may find the following bash commands useful::
+Once you've fixed all merge conflicts, do::
- # Create a conda environment named ``networkx-dev``
- conda create --name networkx-dev
- # Activate it
- conda activate networkx-dev
- # Install major development and runtime dependencies of networkx
- # (the rest can be installed from conda-forge or pip, if needed)
- conda install `for i in requirements/{default,test,doc,extras}.txt; do echo -n " --file $i "; done`
- # Install minimal testing dependencies
- conda install pytest
- # Install networkx from source
- pip install -e . --no-deps
- # Test your installation
- PYTHONPATH=. pytest networkx
+ git commit
+
+.. note::
+
+ Advanced Git users are encouraged to `rebase instead of merge
+ <https://networkx.github.io/documentation/stable/developer/gitwash/development_workflow.html#rebase-on-trunk>`__,
+ but we squash and merge most PRs either way.
Guidelines
@@ -198,14 +228,6 @@ Guidelines
* All changes are reviewed. Ask on the
`mailing list <http://groups.google.com/group/networkx-discuss>`_ if
you get no response to your pull request.
-
-Stylistic Guidelines
---------------------
-
-* Set up your editor to remove trailing whitespace.
- Follow `PEP08 <www.python.org/dev/peps/pep-0008/>`_.
- Check code with `pyflakes` / `flake8`.
-
* Use the following import conventions::
import numpy as np
@@ -214,7 +236,24 @@ Stylistic Guidelines
import matplotlib.pyplot as plt
import networkx as nx
- cimport numpy as cnp # in Cython code
+* Use the decorator ``not_implemented_for`` in ``networkx/utils/decorators.py``
+ to designate that a function doesn't accept 'directed', 'undirected',
+ 'multigraph' or 'graph'. The first argument of the decorated function should
+ be the graph object to be checked.
+
+ .. code-block:: python
+
+ @nx.not_implemented_for('directed', 'multigraph')
+ def function_not_for_MultiDiGraph(G, others):
+ # function not for graphs that are directed *and* multigraph
+ pass
+
+ @nx.not_implemented_for('directed')
+ @nx.not_implemented_for('multigraph')
+ def function_only_for_Graph(G, others):
+ # function not for directed graphs *or* for multigraphs
+ pass
+
Testing
-------
@@ -223,16 +262,11 @@ Testing
execution on your system. The test suite has to pass before a pull
request can be merged, and tests should be added to cover any
modifications to the code base.
-
We make use of the `pytest <https://docs.pytest.org/en/latest/>`__
testing framework, with tests located in the various
``networkx/submodule/tests`` folders.
-To use ``pytest``, ensure that the library is installed in development mode::
-
- $ pip install -e .
-
-Now, run all tests using::
+To run all tests::
$ PYTHONPATH=. pytest networkx
@@ -253,15 +287,10 @@ For example, run all tests and all doctests using::
$ PYTHONPATH=. pytest --doctest-modules networkx
-Test coverage
--------------
-
Tests for a module should ideally cover all code in that module,
i.e., statement coverage should be at 100%.
-To measure the test coverage, install
-`pytest-cov <https://pytest-cov.readthedocs.io/en/latest/>`__
-(using ``pip install pytest-cov``) and then run::
+To measure the test coverage, run::
$ PYTHONPATH=. pytest --cov=networkx networkx
@@ -276,71 +305,6 @@ detailing the test coverage::
networkx/algorithms/approximation/clique.py 42 1 18 1 97%
...
-Pull request codes
-------------------
-
-When you submit a pull request to GitHub, GitHub will ask you for a summary. If
-your code is not ready to merge, but you want to get feedback, please consider
-using ``WIP: experimental optimization`` or similar for the title of your pull
-request. That way we will all know that it's not yet ready to merge and that
-you may be interested in more fundamental comments about design.
-
-When you think the pull request is ready to merge, change the title (using the
-*Edit* button) to remove the ``WIP:``.
-
-.. _deprecation_policy:
-
-
-Deprecation policy
-------------------
-
-If the behavior of the library has to be changed, a deprecation cycle must be
-followed to warn users.
-
-A deprecation cycle is *not* necessary when:
-
-* adding a new function, or
-* adding a new keyword argument to the *end* of a function signature, or
-* fixing buggy behavior
-
-A deprecation cycle is necessary for *any breaking API change*, meaning a
-change where the function, invoked with the same arguments, would return a
-different result after the change. This includes:
-
-* changing the order of arguments or keyword arguments, or
-* adding arguments or keyword arguments to a function, or
-* changing the name of a function, class, method, etc., or
-* moving a function, class, etc. to a different module, or
-* changing the default value of a function's arguments.
-
-Usually, our policy is to put in place a deprecation cycle over two releases.
-
-Note that the 2-release deprecation cycle is not a strict rule and in some
-cases, the developers can agree on a different procedure upon justification
-(like when we can't detect the change, or it involves moving or deleting an
-entire function for example).
-
-Explicitly not supporting directed or multigraph in a function
---------------------------------------------------------------
-
-Use the decorator ``not_implemented_for`` in ``networkx/utils/decorators.py``
-to designate that a function doesn't accept 'directed', 'undirected',
-'multigraph' or 'graph'.
-The first argument of the decorated function should be the graph
-object to be checked.
-
-.. code-block:: python
-
- @nx.not_implemented_for('directed', 'multigraph')
- def function_not_for_MultiDiGraph(G, others):
- # function not for graphs that are directed *and* multigraph
- pass
-
- @nx.not_implemented_for('directed')
- @nx.not_implemented_for('multigraph')
- def function_only_for_Graph(G, others):
- # function not for directed graphs *or* for multigraphs
- pass
Bugs
----