-
Notifications
You must be signed in to change notification settings - Fork 55
Contributing
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.
- Setting Up a Development Environment
- Clone the pyGSTi Repository
- Install Development Dependencies
- Write Your Code
- Write Tests
- Commit Your Changes
- Running Tests
- Update Your Branch
- Push Your Branch
- Submit a Pull Request
- Get some Feedback
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.
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.
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.
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]
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.
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.
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.
To run all tests:
$ nosetests test/test_packages
To run tests for a specific unit:
$ nosetests test/test_packages/my_package/testMyFeature.py
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.
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...
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.
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.
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.
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.
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.
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.
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 in the issue tracker.
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.
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.
(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.
TODO
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.
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-
- Topic branches for bugfixes should have names starting with
- When merging back into
develop
, developers make a pull request which should be reviewed by another developer.
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.
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.
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
pyGSTi uses the numpydoc docstring style convention.
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:
- emacs packages:
- editorconfig
- elpy for flake8 (and other useful tools)
- py-autopep8
- vim plugins:
- editorconfig-vim
- vim-flake8
- Set
formatprg=autopep8
in your vimrc to use autopep8 as a formatter
- Visual Studio Code:
- EditorConfig for VS Code
- If installed, flake8 is enabled by default
- Set
"python.formatting.provider": "autopep8"
to use autopep8 as a formatter
- PyCharm:
- Ships with EditorConfig.
- No flake8 or autopep8 support is available, but may be configured as external tools.
- atom plugins:
- Sublime Text packages:
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.
pyGSTi includes a range of static tests which developers can use to ensure that new changes don't break functionality in unexpected ways.
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.
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.
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.
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.
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:
- Lint the code for critical errors (syntax errors & undefined names)
- Lint the code for style
- This is for information only and will not cause the build to fail
- Run unit tests on all supported python versions (3.5, 3.6, 3.7, 3.8)
- Push the changes to
beta
- This is not run for pull requests
On beta
and master
, TravisCI will:
- Lint the code for both critical errors and style
- Failing this will cause the build to fail
- Run unit tests on all supported python versions (3.5, 3.6, 3.7, 3.8)
- Run system tests on the minimum supported python version (3.5)
Pull requests will run the CI routine for the branch being merged into.