Skip to content

Development

benedikt-voelkel edited this page Jan 9, 2019 · 14 revisions

Instructions for developers

If you want to contribute to this package, the workflow is basically the following

  1. click here to fork the upstream repository
  2. create your development branch <my-awesome-development> on your fork
  3. do the development
  4. make local validation tests
  5. when ready, create a pull request (PR) to the upstream repository's master branch

Coding guidelines

  • Keep your code consistent and concise!
  • Put comments when necessary or useful - this really helps
  • Choose meaningful names for functions and variables, that can sometimes even replace a comment
  • Try to avoid variables in the global scope
  • Use the logger in case you want to add verbosity

Formatting

The formatting guidelines follow the PEP8 coding conventions. You can enforce this style using autopep8 which can be installed using pip. The basic usage is

autopep8 --max-line-length=100 machine_learning_hep

which shows the changes. Note that in some cases pylint might complain since the formatting rules are not 100% identical which can happen in corner cases. In-place changes can be made by adding the --in-place flag. If you do that after committing please squash and don't add an additional 'Enforce format'-like commit.

Using the logger

The package provides a small logger utility based on Python's logging module which should be used whenever you want to add some verbosity to your code. Please avoid messages via print() since everyone would come up with a new format e.g. for error or debug messages. If you want to use it somewhere, the import

from machine_learning_hep.logger import get_logger

will get you the function to get the logger. Obtaining the actual logger object works via

logger = get_logger()

There are 5 levels:

  1. info: logger.info("An info...")
  2. warning: logger.warning("A warning...")
  3. error: logger.error("An error..."), this can be seen as a serious warning
  4. critical: logger.critical("Something went completely wrong..."), the message will be printed and the execution is aborted
  5. debug: logger.debug("A debug message..."), will only be logged if the flag --debug is passed on the command

The logging can be forwarded to a file using the flag --logfile <somelogfile> on the command line.

Creating a new PR

Try to focus on one major extensions/change per PR. A PR can consist of multiple commits. However, keep the history extension concise and don't spoil it with many small commits. The additional history should reflect the main steps of your development. If you have many small commits (which may be the case since you want to keep track locally) you might want to squash some of those together before creating the PR. In that case, have a look here. Rewriting the history is in that case safe because it is only you working on your branch in your fork. Of course, rewriting the history of branches different people work on can be dangerous.

Depending on the scope of your changes more detailed commit message could make sense. In that case provide a short commit message (~80-100 characters) along with a more detailed one pointing out your developments. Especially, when there are changes in the user API that makes a lot of sense. In the latter case, once your PR is merged, please update the Wiki if necessary.

Commenting on PRs

From time to time it might make sense to have a look at open PRs. If you have an idea, comments etc. concerning a specific PR, comment on that.

Open an issue

There are different cases when opening an issue makes sense and can be done here, e.g.:

  • There is a bug: make clear where the bug is and under which circumstances it emerges. If you have suggestions to resolve it, dump these as well.
  • In case you have a nice idea for a useful enhancement of the package, let people know. Even if you don't have time to implement this yourself, you might want to start a discussion on whether that could be useful in general. Then, also other people who might be in charge for certain parts of the code can pick up your ideas.

To have better overview, add corresponding labels to your issue and refer to other issues, code or PRs whenever useful.

Local validations

Make sure that what you implemented makes at least sense in your environment before opening a PR. pylint is used to ensure the code is correctly written and in a uniform format: pylint's configuration can be found in the .pylintrc file in the repository's root directory.

You can run the full validation (the same used to evaluate your PRs) by running the following from the top directory of the project:

ci/run-tests.sh

This may be time-consuming as it will test every Python file in the repository. You can simply run pylint on the files you have changed if you want:

pylint <somefile.py>

If you run it from the root directory of your repository, the configuration file will be picked up automatically.

Continuous Integration

Before accepting any PR, it has to pass the continuous integration tests including what you can check already locally. Merging is blocked in case PRs do not pass the tests.