Skip to content

Contributing

Rob edited this page May 15, 2020 · 29 revisions

Contributing to pyGSTi

This document serves to describe some of the common practices, procedures, and conventions followed by pyGSTi developers. Members of the pyGSTio group should use this document as a guide to making contributions to the pyGSTi codebase.

Don't think of these guidelines as rules. They won't be enforced and are subject to change. Rather, consider them tips on how to make developing pyGSTi easier and more fruitful for everyone involved.

These guidelines do not apply to contributors outside of the pyGSTio group. If you're an external contributor to pyGSTi, please refer to the general contributor guidelines.

Elements of this guide have been adapted from the Ruby on Rails contributor guide.

Contents

Workflow

Minor Patches

Documentation

Conventions

Workflow

This section is a step-by-step guide to contributing code to the pyGSTi project. You don't need to follow these steps to make a contribution to pyGSTi, but if you're not sure how to use the guidelines and conventions of the project, you might want to check the relevant heading in this section.

pyGSTi developers use a simple & flexible branching git workflow. This section will explain in detail how to you can use this workflow when making a contribution to pyGSTi.

Setting Up a Development Environment

pyGSTi developers may use whatever operating system they choose, but out of convenience, this guide will use example commands for Linux and OS X platforms.

If you don't have python 3.7, pip, and git installed, install them using your OS's package manager.

You'll need to have python 3.7 installed. Check your python version and ensure you're using at least 3.7:

$ python3 -V
Python 3.7.2

pyGSTi developers use git to collaborate on development. If you haven't used git before, you may want to take a moment to learn about git -- or at the very least, go through first-time git setup.

You may also want to take a moment to configure your editor or IDE.

Clone the pyGSTi Repository

To be able to contribute code, you need to clone the pyGSTi repository:

$ git clone https://github.com/pyGSTio/pyGSTi.git

pyGSTi uses the unstable develop branch for active development. Switch to it now:

$ cd pyGSTi/
$ git checkout develop

From develop, you should switch to a topic branch for your work. Name it something descriptive, starting with feature- or bugfix- if you're working on a new feature or a bugfix, respectively:

$ git checkout -b feature-my-feature

If your branch is associated with a specific issue in the issue tracker, it can be helpful to prefix the branch name with the number of that issue (i.e. 42-bugfix-fixing-bug for a branch fixing bug #42 in the tracker.) This helps contributors track progress on specific issues, and lets maintainers know when an old branch can be safely deleted.

This branch will only exist locally on your machine until you push it to the remote repository, so don't worry about making changes that could mess up other developers' work.

Install Development Dependencies

You'll most likely want to create and activate a python virtual environment to manage pyGSTi and its dependencies:

$ python3 -m venv local/3.7
$ source local/3.7/bin/activate

(These commands may vary depending on your python version and shell)

pyGSTi has a number of optional dependencies. In order to run all tests, developers should install pyGSTi with all optional dependencies with pip:

$ pip install -e .[complete]

Write Your Code

Now you can get down to business and add/edit code. Since you're on your own branch, you're free to write whatever you want, but if you're planning on submitting your change back for inclusion in pyGSTi, keep a few things in mind:

  • Code pythonically!
  • Document what you write, and update documentation for everything affected by your contribution.
  • Try to follow our conventions for organization, style, and documentation.
  • Include tests that fail without your code, and pass with it.

Write Tests

You should always write tests for any code you write. We use the python built-in unittest module with nose.

pyGSTi keeps its tests under test/test_packages. Add your tests to the appropriate sub-module under test_packages.

Many developers find it useful to write tests first, then write implementations that pass their tests; this is called test-driven development and is a useful strategy, but can be tricky in practice. By no means is test-driven development required to contribute to pyGSTi.

Running Tests

It's important to run regression tests to make sure you haven't accidentally broken existing functionality in pyGSTi. The full suite of tests can take a long time to run, but it's not customary to run all tests before pushing changes. As a compromise, test what your code obviously affects. If all tests pass, that's enough to make a pull request. We use TravisCI to run the full test suite on each pull request as a safety net.

All Tests

To run all tests:

$ nosetests test/test_packages

For a Particular Component

To run tests for a specific unit:

$ nosetests test/test_packages/my_package/testMyFeature.py

Commit Your Changes

When you've written something you're happy with (more-or-less), stage the files with your changes and commit your changes with git:

$ git add packages/pygsti/my_package/*
$ git commit

This will open up your text editor where you can write a commit message describing what you've changed. Use this message to tell other developers what you did.

A good commit message looks like this:

Short summary (ideally 50 characters or less)

More detailed description, if necessary. It should be wrapped to
72 characters. Try to be as descriptive as you can. Even if you
think that the commit content is obvious, it may not be obvious
to others. Add any description that is already present in the
relevant issues; it should not be necessary to visit a webpage
to check the history.

The description section can have multiple paragraphs.

Code examples can be embedded by indenting them with 4 spaces:

    def Y(f):
        def apply(y):
            return f(lambda *args: y(y)(*args))
        return (lambda x: x(x))(apply)
    fac = lambda f: lambda n: (1 if n<2 else n*f(n-1))
    Y(fac)(10)  # 10! = 3628800

You can also add bullet points:

- make a bullet point by starting the line with a dash (-)
  or an asterisk (*)

- wrap lines at 72 characters, and indent any additional lines
  with 2 spaces for readability

Alternatively, for small commits, you can just write a summary message:

$ git commit -a -m "Updated version number to v2.7.1.8"

If you're finding it tiresome to write long commit messages describing all your changes, consider making smaller commits more often.

Undoing Commits

If you commit something and regret it later, you can roll it back:

$ git reset HEAD~

(HEAD~ is a shortcut for "the commit before last". You could go back further with something like HEAD~3 to revert the last three commits.)

Now your last commit is gone. Your changed files are still there, but unstaged. If you want, you can roll back the files, too:

$ git checkout packages/pygsti/my_package

Be careful changing the past, though. Changing the history of a branch is all fine and dandy when it's only on your local machine, but you shouldn't erase commits you've pushed to a remote repository! Other developers might have used your committed changes in their own work. By changing history, you're pulling the rug out from under them! Don't be surprised if you get some angry emails...

Update Your Branch

Somebody else might have made changes to pyGSTi while you were busy. Apply them to your local copy of develop:

$ git checkout develop
$ git pull --rebase

Now reapply your patch on top of the latest changes:

$ git checkout feature-my-feature
$ git rebase develop

Git will tell you if it finds a conflict -- that is, if you and somebody else changed the same thing, and git can't figure out which one should really be there. You'll need to manually go to the conflicting code and resolve the conflict yourself, then continue with git rebase --continue.

Once any conflicts are resolved, you should probably run tests again to make sure the new changes haven't broken your code's functionality. If they pass, your changes are ready to be added to pyGSTi.

Push Your Branch

If you haven't already, push your changes:

$ git push -u origin feature-my-feature

Now your local changes are mirrored in a branch on the remote git repository. You can still make more changes if you want; they won't be sent to the remote branch until the next time you push.

You can push to your topic branch as often as you like -- and if you're looking at a branch another developer is working on, you should be aware that they will push to that branch as often as they like. You usually shouldn't push any changes to develop except under certain circumstances, and ordinary developers cannot push directly to master or beta. If you want to add your changes to these mainline branches, use a pull request.

Submit a Pull Request

Ready to submit your contribution for inclusion in pyGSTi? Create a pull request. This tells other developers you're finished with the thing you're working on and want to merge it into the mainline. Making a pull request also signals to other developers that code in the mainline has changed (or is about to), so pull requests are also a standard way to indicate to warn developers that there will be changes for them to integrate into their own work.

To create a pull request, navigate to the the pyGSTi Github page. Under the Pull requests tab, press New pull request in the upper-right-hand corner. You probably want to merge into develop, so set the base branch to develop and set the compare branch to your topic branch using the pull-down menus, then press Create pull request.

Check over the changesets and make sure they're the changes you want to merge. Fill in some details about your potential patch including a meaningful title. When finished, press Create pull request. The core developers will be notified, and will review your submitted pull request before long.

Get some Feedback

When you make a pull request, a core developer will assign themself to the request and review your patch. They'll make sure it passes tests (and therefore doesn't break necessary functionality) and conforms to pyGSTi's conventions. Code reviews are necessary to make sure that developers don't accidentally break things, but also as a way for the core developers to keep track of the changes happening to pyGSTi.

If everything checks out, your reviewer will then merge your patch into develop (or whichever branch you're merging into). If things don't totally check out, the developer reviewing your pull request will leave a comment with suggestions for changes they'd like you to make. Some suggestions can be matters of opinion -- feel free to discuss these things in the pull request comments. Use your judgment and be reasonable. In any case, once your reviewer is satisfied, they will merge your patch. Great! Now you can go back to step 1. Lather, rinse, repeat.

The core development team is small and has many very important things to do, probably. If it's been a while since you submitted your pull request and no-one's been assigned as a reviewer, shoot them an email to let them know.

Merging Your Own Pull Request

Circumstances can vary. You may have a patch that needs to be merged now, due to, say, time constraints or some other external factor. If you've got a good reason, you can certainly skip the code review and merge it into develop yourself. But don't make a habit of it. Use your judgment.

Minor Patches

For small changes to the pyGSTi codebase, internal contributors are free to push their commits directly to develop instead of making a pull request for review. The purpose of code reviews and pull requests is to make sure that another pyGSTi developer is aware of what you're adding to the codebase, so as a rule-of-thumb: if someone would want to know about what you're adding, make a pull request!

Here are a few examples of changes that probably don't need a PR:

  • Renaming local symbols
  • Fixing a typo
  • Adding or editing a docstring with minor information

Likewise, here are a couple of changes that would definitely need a PR:

  • Refactoring something in a module or class namespace
  • Adding or removing a feature
  • Fixing a bug
  • Making extensive changes to documentation across many units
  • Adding a missing test case or fixing a broken test

There's also another alternative: if you're making a reasonably minor change that other developers should still be made aware of, consider making a pull request and merging it yourself. That way, you can still give other developers a way to keep track of what you've changed.

Documentation

We rely on our contributors to keep our documentation correct, consistent, usable, and up-to-date. If you have additions or corrections to add to our project's documentation, we have a couple of resources available to you.

Issues relating to documentation should be tagged with the label: docs label in the issue tracker.

Code & API Documentation

When possible, pull requests that add features should include the documentation for those features, and any revisions to a documented unit should revise documentation accordingly.

If a function, class, or module has an outdated docstring, please fix it and push the revised documentation directly to develop. The only exceptions to this are patches adding or fixing docstrings for a large number of units, which should be accompanied by a pull request -- use your judgment.

Docstrings in pyGSTi will be parsed to compile the project's documentation on Read the Docs. When adding or editing docstrings, pay close attention to our docstring style conventions.

User Guides, Examples, & Tutorials

pyGSTi ships with a number of interactive tutorials and examples in the form of Jupyter notebooks found in the jupyter_notebooks directory.

Developers are not expected to add tutorials or examples when adding features, but may do so if they wish.

Developer Wiki

(YOU ARE HERE)

The project developer wiki is a repository of general information about pyGSTi. It's sort of a catch-all for documentation not covered anywhere else.

When adding new wiki pages or editing existing documentation, let other developers know by creating a new issue in the issue tracker and assigning yourself.

If you are creating a new major section for the wiki, you should manually add it to the navigation menu.

Publications

TODO

Conventions

This section details a few of the common development conventions followed by pyGSTi developers. These are not absolute requirements for contributing, but in some rare cases, contributions to the codebase may be temporarily rejected for breaking from conventions. We really appreciate contributors making an effort to follow these conventions and keep our codebase clean, clear, and maintainable.

Repository

The pyGSTi repository uses three mainline branches:

  • master -- the stable release branch
  • beta -- the unstable pre-release branch
  • develop -- the unstable development branch

Developers merge their contributions into develop. When changes are pushed to develop, CI will lint the branch for syntax errors and run unit tests on the development python version (3.7). If this CI build succeeds, the changes will be automatically pushed to beta. When the pyGSTi core developers make a new release, they will manually merge it from beta into master and push a new tag.

pyGSTi developers use a typical branching workflow. To learn more, the git book has a relevant section. But the gist of it is this:

  • Developers branch off of develop when creating topic branches
    • Topic branches for bugfixes should have names starting with bugfix-
    • Topic branches for features & enhancements should likewise start with feature-
  • When merging back into develop, developers make a pull request which should be reviewed by another developer.

Releases

Releases of pyGSTi are hosted on PyPI and are numbered using semantic versioning. Developers should follow the release guide when preparing to deploy a new release of pyGSTi.

Style

Keeping pyGSTi written in a consistent style makes code easier to read, easier to understand, easier to debug, and easier to maintain. Pull requests will not be rejected for failing to meet style guidelines; TravisCI will lint style on pull requests, but a failed style lint will not fail the build. However, releases are required to meet style guidelines, so your code may be automatically formatted before being included in a release.

No one would reasonably expect any developer to code with consistent style on their own. Instead, we strongly encourage developers to configure their editors with the configurations we supply.

Code Style

pyGSTi follows the PEP8 style guide, with a few notable exceptions:

Code Error description Reason for ignoring
E265 block comment should start with '# ' autopep8 will not fix instances of commented-out code
E266 too many leading '#' for block comment Useful for indicating commented-out code
E402 module level import not at top of file Used for a few workarounds to python 2-3 compatibility
E501* line too long (> 79 characters) Extended limit to 120 characters, see footnote
E701 multiple statements on one line (colon) More readable for scientific programming
E702 multiple statements on one line (semicolon) ''
E704 multiple statements on one line (def) ''
E722 do not use bare except, specify exception instead Mostly harmless
E741 do not use variables named 'l', 'O', or 'I' Mostly harmless
W503 line break before binary operator Mutually exclusive with W504 (after binary operator)
W605 invalid escape sequence 'x' Used in LaTeX samples
F401 module imported but unused Major namespace refactor planned, TBD
F403 'from module import *' used; unable to detect undefined names ''
F405 name may be undefined, or defined from star imports: module ''

*still enforced, with an extended length limit of 120 characters

Docstring Style

pyGSTi uses the numpydoc docstring style convention.

Useful Tools

pyGSTi includes configuration for flake8 linting and editorconfig, which can be used by your editor to highlight errors and automatically apply basic formatting on save. We recommend all pyGSTi contributors take 5 minutes to configure their editors to use the included configurations.

If you use one of the following editors, there are a few packages or plugins that you may want to install to use the configurations we supply:

pyGSTi also includes an auto-formatter script that wraps autopep8. Developers can use this to automatically format a given file or module. Be advised that this script will only fix minor style errors like whitespace. Semantic errors must be fixed manually.

Testing

pyGSTi includes a range of static tests which developers can use to ensure that new changes don't break functionality in unexpected ways.

Unit Tests

Most pyGSTi units should have unit tests. Generally, unit tests should:

  • Assert tested code is correct by checking the results of a test against an expected state
  • Assert tested code is complete by testing all expected functionality of the covered unit
  • Avoid testing functionality of other units by mocking where applicable
  • Check that the covered unit raises any and all expected exceptions whenever it should
  • Cover edge cases
  • Run fairly quickly (usually no more than 0.1 seconds per test)

Some things don't need unit tests. These include:

  • Throw-away or placeholder code
  • Code interacting with external systems (see integration tests)
  • GUI code
  • Jupyter notebooks

But the above list is not exhaustive. Use your judgment, and when in doubt, test it.

Patches submitted as pull requests should add unit tests for new functionality and change unit tests for changing functionality. Code that doesn't need unit tests should be excluded from coverage analysis.

Functional Tests

In addition to unit tests, pyGSTi contains functional tests, which simply check that a particular use case works as expected. Functional tests treat the underlying unit(s) as a "black box" and should avoid mocking any functionality below the tested case.

Functional tests are not required, but developers may wish to add them as an additional safety net to catch future changes altering the way a user interacts with their code.

Integration Tests

Any new feature that relies on an external system or API should include integration tests and a documented test procedure.

For now, pyGSTi has no such features, nor is it expected to.

Regression Tests

pyGSTi has several objects that it can serialize to the disk. As the project grows, those file formats change, so tests are needed to check if things serialized with an old version can still be read. We call these "regression tests," which is a small but convenient abuse of terminology.

Patches that cause regression tests to fail should update the legacy I/O adapter in order to be able to load older versions.

Continuous Integration

pyGSTi uses TravisCI to automatically run tests and generate reports. When code is pushed to the pyGSTi repository, a new CI build is started automatically. The specific script run by CI depends on the branch.

On develop, TravisCI will:

  1. Lint the code for critical errors (syntax errors & undefined names)
  2. Lint the code for style
    • This is for information only and will not cause the build to fail
  3. Run unit tests on all supported python versions (3.5, 3.6, 3.7, 3.8)
  4. Push the changes to beta
    • This is not run for pull requests

On beta and master, TravisCI will:

  1. Lint the code for both critical errors and style
    • Failing this will cause the build to fail
  2. Run unit tests on all supported python versions (3.5, 3.6, 3.7, 3.8)
  3. Run system tests on the minimum supported python version (3.5)

Pull requests will run the CI routine for the branch being merged into.