Contribute to beat.editor

Development environment setup

Follow the same procedure as other beat package to setup a conda environment ready for development.

  1. Setup conda environment:

    $ /path/to/bob.admin/conda/conda-bootstrap.py --overwrite --python=3.6 beat_editor_env
    
  2. Setup development environment:

    $ buildout -c development.cfg
    
  3. Install the pre-commit package:

    $ conda install pre_commit
    
  4. Setup pre-commit package:

    $ pre-commit install
    
  5. You can run the pre-commit hooks at any time by calling:

    $ pre-commit run --all-files
    

beat.editor needs the following dependencies for its pre-commit hooks:

  • black

  • flake8

  • bandit

black is currently available through Idiap’s conda channel.
flake8 is available through conda’s defaults channel.
bandit is not yet available on either so it must be installed using pip:
$ pip install bandit

If you would like to install them all using pip you can call:

$ pip install -r pre-commit-dependencies.txt

Development

Writing code

beat.editor is a python application or rather Qt Python application. It will use the QtWidgets module for the GUI part.

The current implementation will use PyQt as Qt for Python is not yet available on conda.

While the code is written in Python the API is mostly the same as the C++ framework minus some pythonism that can be found in the PyQt documentation.

Writing tests

beat.editor uses pytest as its testing framework because of the pytest-qt module that provides support for testing Qt based application.

An example of tests is shown here:

# vim: set fileencoding=utf-8 :
###############################################################################
#                                                                             #
# Copyright (c) 2019 Idiap Research Institute, http://www.idiap.ch/           #
# Contact: beat.support@idiap.ch                                              #
#                                                                             #
# This file is part of the beat.editor module of the BEAT platform.           #
#                                                                             #
# Commercial License Usage                                                    #
# Licensees holding valid commercial BEAT licenses may use this file in       #
# accordance with the terms contained in a written agreement between you      #
# and Idiap. For further information contact tto@idiap.ch                     #
#                                                                             #
# Alternatively, this file may be used under the terms of the GNU Affero      #
# Public License version 3 as published by the Free Software and appearing    #
# in the file LICENSE.AGPL included in the packaging of this file.            #
# The BEAT platform is distributed in the hope that it will be useful, but    #
# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY  #
# or FITNESS FOR A PARTICULAR PURPOSE.                                        #
#                                                                             #
# You should have received a copy of the GNU Affero Public License along      #
# with the BEAT platform. If not, see http://www.gnu.org/licenses/.           #
#                                                                             #
###############################################################################

import pytest

from PyQt5 import QtCore
from PyQt5.QtCore import QStringListModel
from PyQt5.QtCore import pyqtSlot
from PyQt5.QtWidgets import QPushButton

from ..backend.asset import AssetType
from ..widgets.editor import AbstractAssetEditor
from ..widgets.field import FieldWidget


class MockAssetEditor(AbstractAssetEditor):
    """
    Mock editor to show how to test an editor
    """

    def __init__(self, parent=None):
        super().__init__(AssetType.UNKNOWN, parent)
        self.dataformat_model = None
        self.add_field_button = QPushButton(self.tr("Add"))
        self.layout().addWidget(self.add_field_button)

        self.add_field_button.clicked.connect(self.__addField)

    @pyqtSlot()
    def __addField(self):
        self.layout().addWidget(FieldWidget(self.dataformat_model))

    def set_dataformat_model(self, model):
        self.dataformat_model = model

    def _load_json(self, json_object):
        """Load the json object passed as parameter"""

        for name, type_ in json_object["field"].items():
            field = FieldWidget(self.dataformat_model)
            field.format_name = name
            field.format_type = type_
            self.layout().addWidget(field)

    def _dump_json(self):
        """Returns the json representation of the asset"""

        json_data = {}

        field_list = self.findChildren(FieldWidget)
        json_data["field"] = {}
        for field in field_list:
            json_data["field"][field.format_name] = field.format_type

        return json_data


@pytest.fixture()
def dataformat_model():
    return QStringListModel(["float32", "float64", "int32", "int64"])


class TestMockEditor:
    """Test that the mock editor works correctly"""

    def test_json_load_and_dump_field_widget(self, qtbot, dataformat_model):
        json_reference = {
            "field": {"value32": "float32", "value64": "float64"},
            "description": "Short description test",
        }

        widget = MockAssetEditor()
        widget.set_dataformat_model(dataformat_model)
        widget.load_json(json_reference)

        assert widget.dump_json() == json_reference

    def test_json_load_and_dump_long_description_field_widget(
        self, qtbot, dataformat_model
    ):
        json_reference = {
            "field": {"value32": "float32", "value64": "float64"},
            "description": "This is a very long description to test if the limitation works properly as expected by the editor or not.",
        }

        json_expected_output = {
            "field": {"value32": "float32", "value64": "float64"},
            "description": "This is a very long description to test if the limitation works properly as expected by the editor o",
        }

        widget = MockAssetEditor()
        widget.set_dataformat_model(dataformat_model)
        widget.load_json(json_reference)

        assert widget.dump_json() == json_expected_output

    def test_dataformat_creation_field_widget(self, qtbot, dataformat_model):
        widget = MockAssetEditor()
        widget.set_dataformat_model(dataformat_model)

        qtbot.mouseClick(widget.add_field_button, QtCore.Qt.LeftButton)
        fields = widget.findChildren(FieldWidget)

        assert len(fields) == 1

        field = fields[0]

        field.format_name = "value32"
        field.format_type = "float32"

        assert widget.dump_json() == {"field": {"value32": "float32"}}

        qtbot.mouseClick(widget.add_field_button, QtCore.Qt.LeftButton)
        fields = widget.findChildren(FieldWidget)

        assert len(fields) == 2

        field = fields[1]

        field.format_name = "value64"
        field.format_type = "float64"

        assert widget.dump_json() == {
            "field": {"value32": "float32", "value64": "float64"}
        }

Tests can fall in to categories:

  • Automatic tests

  • Manual tests

Automatic tests are tests that can be fully automated using pytest and pytest-qt while manual tests are tests than requires that a person takes a look at the output of said test. For example check that the rendering of some GUI element is the expected output. This kind of test may require writing a minimal application that will be as automated as possible in order for the operator to only have to do the manual check(s) required.

Run tests

beat.editor uses pytest to run its test suite.

$ pytest -sv

Will run all the tests verbosely and not capture the outputs of the test. The -s option is mandatory if there’s a need to brake in the code using python’s debugger.

If you have built your packages using buildout.cfg and wish to transition to a development stage using development.cfg first make sure you have removed beat.core, beat.cmdline and beat.backend.python from your conda environment

$ conda list # check
$ conda remove <package>

Then rebuild all packages (you can delete the bin/ folder to make sure it’s rebuilt):

$ buildout -c development.cfg

Now running your tests would require pytest to use the current environment and use your dependency packages:

$ ./bin/python -m pytest -sv

Coding guidelines

Coding style

PEP8 rules should be followed when writing code for this module.

In order to avoid having to nitpick about code style. black for python as well as flake8 will be used to ensure that formatting is enforced and doesn’t let the developer wondering if he did it the right way.

Pre-commit hooks will be run to ensure that.

While developing, the hooks can be manually called:

$ pre-commit run --all-files

This command will run the hooks on the whole repository. However more fine grained control can be applied.

Commit guidelines

Commit content

Commits should be atomic. What atomic means is that one commit should target one specific change.

For example:

  1. Introduce new API with corresponding tests

  2. Implement use of new API in module1 with corresponding tests

  3. Implement use of new API in module2 with corresponding tests

The same goes for fixes in, for example, the documentation. If the change is not related to code changes committed at the same time, then it should be a separate commit. This will avoid confusion and also allow for easier refactoring if needed.

Commit message format

The commit message format is the following:

One summary line with the concerned module in brackets One blank line Longer multi-line description of what change and why it was changed. One blank line Meta information like bug fixed by the commit

Example:

[widgets] Automatically update json preview

The preview content was only loaded at the same time as the file parsed.
Now changes to the file are automatically detected and the preview
refreshed.

Fixes #10

If the change is trivial like fixing a typo in a variable name, the long description may be omitted.

Commit message must be meaningful.

For example:

[dependencies] Updated version

is not acceptable as it doesn’t provide any useful information beside “something has been update”. This means that any developer going through the history log will have to open that specific commit to see exactly what changed and may only guess why it has changed.

On the other hand:

[dependencies] Updated minimal foo version to X.Y.Z

Needed because of bug XXXXX

is concise and makes it clear what happened and why.

Branching guidelines

Branching must be used for new features as well as bug fixes. Unless an exceptional circumstance occurs, one branch should implement one feature or one bug fix. If bug fixes or features depend on one of these branch, then a new branch should be created from there and then rebased as soon as the base branch was merged into master.

Before merging occurs, each branch should be rebased on the latest master.

As for the naming, branches should have meaningful names also.

For example:

issue_20_fix_wrong_index
implement_dataformat_editor

A branch doesn’t need to be one commit, it can be but it can also span as many commits as needed in order to implement a feature or bug fix.

Note that it may also happen that one branch contains fixes for several issues as they might be related.

Merge message

Merge messages should again be meaningful and concise about the what and the why.