From e49dcbcf1a5ec882c994fc35228951f82e9e5942 Mon Sep 17 00:00:00 2001
From: Robert Rosca <robert.rosca@xfel.eu>
Date: Wed, 24 Feb 2021 21:11:40 +0100
Subject: [PATCH] commit b132f8c Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Wed Feb 24 21:04:47 2021 +0100
Add link to wiki dev guidelines to readme
commit 0d902b2
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 22:25:40 2021 +0100
Add .git-blame-ignore-revs file preemptively
commit 9e6030a
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 18:55:55 2021 +0100
Fix import order
commit d7ca8e9
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 18:52:24 2021 +0100
Add notes on skipping checks, set to allow failure true, break import order again
commit a6f4324
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 18:38:08 2021 +0100
Fix import order
commit 2854f22
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 18:08:35 2021 +0100
Add accidentally removed dependencies
commit 9ecbc83
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 18:06:45 2021 +0100
Add additional details to the readme
commit 6342f2e
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 16:33:47 2021 +0100
Export SHA var as it's used during pre-commit itself
commit a142df2
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 15:50:24 2021 +0100
Swap lines to make more sense
commit 95183eb
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 15:50:09 2021 +0100
Add a script for easy execution of diff-only pre-commit checks
commit b98fed7
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 14:11:13 2021 +0100
Add todos, adjust headings
commit d6b83d8
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 13:49:16 2021 +0100
Update README.rst
Change to be compliant with RSTcheck
Fix some heading orders and enumerated lists
Update the setup instructions
commit db768d6
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 12:12:29 2021 +0100
Sort requirements
commit 43122cd
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 22 12:12:06 2021 +0100
Add pre-commit to requirements
commit 797af5d
Author: Cyril Danilevski <cyril.danilevski@xfel.eu>
Date: Tue Feb 9 11:19:30 2021 +0100
blabla
commit 6179aaa
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 8 11:22:54 2021 +0100
Add comments to clarify non-standard sections
commit cae49ef
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 8 11:12:19 2021 +0100
Use consistent isort version
commit c4d550a
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 8 11:09:45 2021 +0100
Check if MR commit SHA is set
commit aceded4
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 8 10:52:12 2021 +0100
Add pre-commit configuration
Pretty standard setup
Worth mentioning that flake8 is set to only run on the diff between current
branch and merge target
This will not work outside of a merge request
commit c5b8d36
Author: Robert Rosca <robert.rosca@xfel.eu>
Date: Mon Feb 8 10:50:37 2021 +0100
Update CI to work with pre-commit
Check stage tries to find the commit hash being merged into
Uses that hash to get a list of changed files
Passes this list of files to pre-commit so that checks only run on
modified files
---
.git-blame-ignore-revs | 7 ++
.gitlab-ci.yml | 24 +++-
.pre-commit-config.yaml | 48 ++++++++
README.rst | 258 +++++++++++++++++++++++++++-------------
bin/pre-commit-diff.sh | 12 ++
requirements.txt | 11 +-
6 files changed, 266 insertions(+), 94 deletions(-)
create mode 100644 .git-blame-ignore-revs
create mode 100644 .pre-commit-config.yaml
create mode 100755 bin/pre-commit-diff.sh
diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs
new file mode 100644
index 000000000..d6830b909
--- /dev/null
+++ b/.git-blame-ignore-revs
@@ -0,0 +1,7 @@
+# This file contains info on which commits to ignore for git blame to work
+# correctly, you can use either of these see the 'correct' blame results:
+#
+# - `git blame file.py --ignore-revs-file .git-blame-ignore-revs`
+# - `git config blame.ignoreRevsFile .git-blame-ignore-revs`
+#
+# Second option is a bit better as it'll work on the whole repo all the time
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 687b779fc..04d8c2ef1 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,11 +1,27 @@
-isort:
- stage: test
+stages:
+ - check
+ - test
+
+checks:
+ stage: check
+ only: [merge_requests]
+ allow_failure: true
script:
- - python3 -m pip install --user isort==5.6.4
- - isort --diff **/*.py && isort -c **/*.py
+ - export PATH=/home/gitlab-runner/.local/bin:$PATH
+ # We'd like to run the pre-commit hooks only on files that are being
+ # modified by this merge request, however
+ # `CI_MERGE_REQUEST_TARGET_BRANCH_SHA` is a 'premium' feature according to
+ # GitLab... so this is a workaround for extracting the hash
+ - export CI_MERGE_REQUEST_TARGET_BRANCH_SHA=$(git ls-remote origin $CI_MERGE_REQUEST_TARGET_BRANCH_NAME | cut -d$'\t' -f1)
+ - export FILES=$(git diff $CI_COMMIT_SHA $CI_MERGE_REQUEST_TARGET_BRANCH_SHA --name-only | tr '\n' ' ')
+ - python3 -m pip install --user -r requirements.txt
+ - echo "Running pre-commit on diff from $CI_COMMIT_SHA to $CI_MERGE_REQUEST_TARGET_BRANCH_SHA ($CI_MERGE_REQUEST_TARGET_BRANCH_NAME)"
+ # Pass list of modified files to pre-commit so that it only checks them
+ - echo $FILES | xargs pre-commit run --color=always --files
pytest:
stage: test
+ only: [merge_requests]
script:
- python3 -m pip install --user -r requirements.txt
- python3 -m pip install --user pytest
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
new file mode 100644
index 000000000..da0c4977b
--- /dev/null
+++ b/.pre-commit-config.yaml
@@ -0,0 +1,48 @@
+repos:
+ - repo: meta
+ hooks:
+ - id: identity
+ - repo: https://github.com/nbQA-dev/nbQA
+ rev: 0.3.6
+ hooks:
+ - id: nbqa-isort
+ additional_dependencies: [isort==5.6.4]
+ args: [--nbqa-mutate]
+ - id: nbqa-flake8
+ additional_dependencies: [flake8==3.8.4]
+ args: [--nbqa-mutate]
+ - repo: https://github.com/kynan/nbstripout
+ rev: 0.3.9
+ hooks:
+ - id: nbstripout
+ - repo: https://github.com/pycqa/isort
+ rev: 5.6.4
+ hooks:
+ - id: isort
+ - repo: https://gitlab.com/pycqa/flake8
+ rev: 3.8.4
+ hooks:
+ - id: flake8
+ # If `CI_MERGE_REQUEST_TARGET_BRANCH_SHA` env var is set then this will
+ # run flake8 on the diff from the current commit to the latest commit of
+ # the branch being merged into, otherwise it will run flake8 as it would
+ # usually execute via the pre-commit hook
+ entry: bash -c 'if [ -z ${CI_MERGE_REQUEST_TARGET_BRANCH_SHA} ]; then (flake8 "$@"); else (git diff $CI_MERGE_REQUEST_TARGET_BRANCH_SHA | flake8 --diff); fi' --
+ - repo: https://github.com/myint/rstcheck
+ rev: 3f92957478422df87bd730abde66f089cc1ee19b # commit where pre-commit support was added
+ hooks:
+ - id: rstcheck
+ - repo: https://github.com/pre-commit/pre-commit-hooks
+ rev: v2.3.0
+ hooks:
+ - id: check-added-large-files
+ - id: check-ast
+ - id: check-json
+ - id: check-yaml
+ - id: check-toml
+ - id: end-of-file-fixer
+ - id: trailing-whitespace
+ - id: check-docstring-first
+ - id: check-merge-conflict
+ - id: mixed-line-ending
+ args: [--fix=lf]
diff --git a/README.rst b/README.rst
index 6328ee618..4a9cbb379 100644
--- a/README.rst
+++ b/README.rst
@@ -1,146 +1,234 @@
+###################
Offline Calibration
-===================
+###################
The offline calibration is a package that consists of different services,
responsible for applying most of the offline calibration and characterization
for the detectors.
-Offline calibration installation
-================================
+.. contents::
-It's recommended to install the offline calibration (pycalibration) package
-over maxwell, using anaconda/3 environment.
-Installation using Anaconda
----------------------------
+Offline Calibration Installation
+********************************
-First you need to load the anaconda/3 environment through::
+It's recommended to install the offline calibration (pycalibration) package over
+maxwell, using anaconda/3 environment.
- 1. module load anaconda/3
-If installing into other python enviroments, this step can be skipped.
+Installation using python virtual environment - recommended
+===========================================================
-Then the package for the offline calibration can be obtained from the git repository::
+1. ``git clone ssh://git@git.xfel.eu:10022/detectors/pycalibration.git && cd pycalibration`` - clone the offline calibration package from EuXFEL GitLab
+2. ``module load anaconda/3`` - load the anaconda/3 environment
+3. ``python3 -m venv .venv`` - create the virtual environment
+4. ``source .venv/bin/activate`` - activate the virtual environment
+5. ``python3 -m pip install --upgrade pip`` - upgrade version of pip
+6. ``python3 -m pip install -r requirements.txt`` - install dependencies
+7. ``python3 -m pip install .`` - install the pycalibration package (add ``-e`` flag for editable development installation)
+8. ``pip install "git+ssh://git@git.xfel.eu:10022/karaboDevices/pyDetLib.git#egg=XFELDetectorAnalysis&subdirectory=lib"``
- 2. git clone https://git.xfel.eu/gitlab/detectors/pycalibration.git
+Copy/paste script:
+.. code::
-You can then install all requirements of this tool chain in your home directory by running::
+ git clone ssh://git@git.xfel.eu:10022/detectors/pycalibration.git
+ cd pycalibration
+ module load anaconda/3
+ python3 -m venv .venv
+ source .venv/bin/activate
+ python3 -m pip install --upgrade pip
+ python3 -m pip install -r requirements.txt
+ python3 -m pip install . # `-e` flag for editable install
+ python3 -m pip install "git+ssh://git@git.xfel.eu:10022/karaboDevices/pyDetLib.git#egg=XFELDetectorAnalysis&subdirectory=lib/"
- 3. pip install -r requirements.txt . --user
-in pycalibration's root directory.
+Installation into user home directory
+=====================================
-After installation, you should make sure that the home directory is in the PATH environment variable::
+1. ``git clone ssh://git@git.xfel.eu:10022/detectors/pycalibration.git && cd pycalibration`` - clone the offline calibration package from EuXFEL GitLab
+2. ``module load anaconda/3`` - load the anaconda/3 environment. If installing into other python environments, this step can be skipped
+3. ``pip install -r requirements.txt`` - install all requirements of this tool chain in your home directory
+4. ``pip install .`` - install the pycalibration package (add ``-e`` flag for editable development installation)
+5. ``export PATH=$HOME/.local/bin:$PATH`` - make sure that the home directory is in the PATH environment variable
- 4. export PATH=$HOME/.local/bin:$PATH
+Copy/paste script:
-Installation using virtual python environment
----------------------------------------------
+.. code::
-Create virtual environment::
+ git clone ssh://git@git.xfel.eu:10022/detectors/pycalibration.git
+ cd pycalibration
+ module load anaconda/3
+ pip install -r requirements.txt --user
+ pip install . # `-e` flag for editable install, e.g. `pip install -e .`
+ export PATH=$HOME/.local/bin:$PATH
- module load anaconda/3
- python -m venv /path/to/new/virtual/environment
- source /path/to/new/virtual/environment/bin/activate
-Clone from git::
+Creating an ipython kernel for virtual environments
+===================================================
- cd /path/to/packages
- git clone https://git.xfel.eu/gitlab/detectors/pycalibration.git
- cd pycalibration
+To create an ipython kernel with pycalibration available you should (if using a
+venv) activate the virtual environment first, and then run:
-Install the package::
+.. code::
- pip install -r requirements.txt
+ python3 -m pip install ipykernel # If not using a venv add `--user` flag
+ python3 -m ipykernel install --user --name pycalibration --display-name "pycalibration" # If not using a venv pick different name
-In additional install pyDetLib package, which is required for many notebooks::
+This can be useful for Jupyter notebook tools as https://max-jhub.desy.de/hub/login
- cd /path/to/packages
- git clone https://git.xfel.eu/gitlab/karaboDevices/pyDetLib.git
- cd pyDetLib/lib
- pip install -r requirements.txt
- pip install .
-++++++++++++++++++++++++++++++++++++++++++++++++++
-Setting an ipython kernel for virtual environments
-++++++++++++++++++++++++++++++++++++++++++++++++++
+Contributing
+************
-To set a kernel for your virtual environment::
+Guidelines
+==========
- source /path/to/new/virtual/environment/bin/activate
- pip install ipykernel
- python -m ipykernel install --user --name <virtenv-name> --display-name "virtenv-display-name"
+Development guidelines can be found on the GitLab Wiki page here: https://git.xfel.eu/gitlab/detectors/pycalibration/wikis/GitLab-Guidelines
-This can be useful for Jupyter notebook tools as "max-jhub.desy.de".
+Basics
+======
-Development Installation
+The installation instructions above assume that you have set up SSH keys for use
+with GitLab to allow for passwordless clones from GitLab, this way it's possible
+to run ``pip install git+ssh...`` commands and install packages directly from
+GitLab.
+
+To do this check the settings page here: https://git.xfel.eu/gitlab/profile/keys
+
+Pre-Commit Hooks
+================
+
+This repository uses pre-commit hooks automatically run some code quality and
+standard checks, this includes the following:
+
+a. ``identity`` - The 'identity' meta hook prints off a list of files that the hooks will execute on
+b. 'Standard' file checks
+
+ 1. ``check-added-large-files`` - Ensures no large files are committed to repo
+ 2. ``check-ast`` - Checks that the python AST is parseable
+ 3. ``check-json`` - Checks json file formatting is parseable
+ 4. ``check-yaml`` - Checks yaml file formatting is parseable
+ 5. ``check-toml`` - Checks toml file formatting is parseable
+ 6. ``rstcheck`` - Checks rst file formatting is parseable
+ 7. ``end-of-file-fixer`` - Fixes EoF to be consistent
+ 8. ``trailing-whitespace`` - Removes trailing whitespaces from lines
+ 9. ``check-merge-conflict`` - Checks no merge conflicts remain in the commit
+ 10. ``mixed-line-ending`` - Fixes mixed line endings
+
+c. Code checks
+
+ 1. ``flake8`` - Code style checks
+ 2. ``isort`` - Sorts imports in python files
+ 3. ``check-docstring-first`` - Ensures docstrings are in the correct place
+
+d. Notebook checks
+
+ 1. ``nbqa-flake8`` - Runs flake8 on notebook cells
+ 2. ``nbqa-isort`` - Runs isort on notebook cells
+ 3. ``nbstripoutput`` - Strips output from ipynb files
+
+To install these checks, set up you environment as mentioned above and then run
+the command:
+
+.. code::
+
+ pre-commit install-hooks
+
+This will set up the hooks in git locally, so that each time you run the command
+``git commit`` the hooks get executed on the **staged files only**, beware that
+if the pre-commit hooks find required changes some of them will **modify your
+files**, however they only modify the current working files, not the ones you
+have already staged. This means that you can look at the diff between your
+staged files and the ones that were modified to see what changes are suggested.
+
+
+Run Checks Only On Diffs
------------------------
-For a development installation, which automatically
-picks up (most) changes, first install the dependencies as above,
-but then install the tool-chain separately in development mode (install in home directory using --user, in case of using Anaconda/3)::
+Typically ``pre-commit`` is ran on ``--all-files`` within a CI, however as this
+is being set up on an existing codebase these checks will always fail with a
+substantial number of issues. Using some creative workarounds, the CI has been
+set up to only run on files which have changed between a PR and the target
+branch.
+
+If you want to run the pre-commit checks as they would run on the CI, then you
+can use the ``bin/pre-commit-diff.sh`` to execute the checks as on the CI
+pipeline.
+
+A side effect of this is that the checks will run on **all** of the differences
+between the 'local' and target branch. This means that if changes have recently
+been merged into the target branch, and there is divergence between the two,
+then the tests will run on all the differences.
- pip install -e .
+If this happens and the hooks in the CI (or via the script) run on the wrong
+files then you should **rebase onto the target branch** to prevent the checks
+from running on the wrong files/diffs.
-Activate Offline calibration
-============================
+Skipping Checks
+---------------
-For using pycalibration package one needs to activate it through::
+If the checks are failing and you want to ignore them on purpose then you have two options:
- source activate
+- use the ``--no-verify`` flag on your ``git commit`` command to skip them, e.g. ``git commit -m "Commit skipping hooks" --no-verify``
+- use the variable ``SKIP=hooks,to,skip`` before the git commit command to list hooks to skip, e.g. ``SKIP=flake8,isort git commit -m "Commit skipping only flake8 and isort hooks"``
-from inside of the pycalibration directory. This will automatically load
-all needed modules and export the $PATH for the home directory.
+In the CI pipeline the pre-commit check stage has ``allow_failure: true`` set so
+that it is possible to ignore errors in the checks, and so that subsequent
+stages will still run even if the checks have failed. However there should be a
+good reason for allowing the checks to fail, e.g. checks failing due to
+unmodified sections of code being looked at.
Python Scripted Calibration
-===========================
+***************************
-First: do not run this on the Maxwell gateway. Rather, `salloc`
-a node for yourself first::
+**Do not run this on the Maxwell gateway**. Rather, ``salloc`` a node for
+yourself first:
- salloc -p exfel/upex -t 01:00:00
+.. code::
-where `-p` gives the partition to use: exfel or upex and `-t`
-the duration the node should be allocated. Then `ssh` onto
-that node.
+ salloc -p exfel/upex -t 01:00:00
-(optionally) Set up the environment::
+where `-p` gives the partition to use: exfel or upex and `-t` the duration the
+node should be allocated. Then `ssh` onto that node.
- module load python3
- pip install --user ipython --upgrade
- pip install --user ipyparallel --upgrade
- pip install --user dill
-
-If running headless (i.e. without X forwarding), be sure to set
-`MPLBACKEND=Agg`, via::
+Then activate your environment as described above (or just continue if you are
+not using a venv).
- export MPLBACKEND=Agg
+If running headless (i.e. without X forwarding), be sure to set
+``MPLBACKEND=Agg``, via:
-Then start an `ipcluster`. If you followed the steps above this can be done
-via::
+.. code::
- ~/.local/bin/ipcluster start --n=32
+ export MPLBACKEND=Agg
+Then start an ``ipcluster``. If you followed the steps above this can be done
+via:
-Run the script::
+.. code::
+
+ ipcluster start --n=32
+
+
+Finally run the script:
+
+.. code::
python3 calibrate.py --input /gpfs/exfel/exp/SPB/201701/p002012/raw/r0100 \
- --output ../../test_out --mem-cells 30 --detector AGIPD --sequences 0,1
+ --output ../../test_out --mem-cells 30 --detector AGIPD --sequences 0,1
Here `--input` should point to a directory of `RAW` files for the detector you
-are calibrating. They will be output into the folder specified by `--output`,
-which will have the run number or the last folder in the hiearchy of the input
-appended. Additionally, you need to specify the number of `--mem-cells` used
-for the run, as well as the `--detector`. Finally, you can optionally
-specify to only process certain `--sequences` of files, matching the sequence
-numbers of the `RAW` input. These should be given as a comma-separated list.
-
-Finally, there is a `--no-relgain` option, which disables relative gain
+are calibrating. They will be output into the folder specified by `--output`,
+which will have the run number or the last folder in the hierarchy of the input
+appended. Additionally, you need to specify the number of `--mem-cells` used for
+the run, as well as the `--detector`. Finally, you can optionally specify to
+only process certain `--sequences` of files, matching the sequence numbers of
+the `RAW` input. These should be given as a comma-separated list.
+
+Finally, there is a `--no-relgain` option, which disables relative gain
correction. This can be useful while we still further characterize the detectors
-to provid accurate relative gain correction constants.
+to provide accurate relative gain correction constants.
You'll get a series of plots in the output directory as well.
-
diff --git a/bin/pre-commit-diff.sh b/bin/pre-commit-diff.sh
new file mode 100755
index 000000000..9219bf916
--- /dev/null
+++ b/bin/pre-commit-diff.sh
@@ -0,0 +1,12 @@
+#!/bin/bash
+
+# We'd like to run the pre-commit hooks only on files that are being modified by
+# this merge request, however `CI_MERGE_REQUEST_TARGET_BRANCH_SHA` is a'premium'
+# feature according to GitLab... so this is a workaround for extracting the hash
+CI_MERGE_REQUEST_TARGET_BRANCH_NAME="${1:-master}" # Set to master or 1st input
+CI_COMMIT_SHA=$(git rev-parse HEAD)
+export CI_MERGE_REQUEST_TARGET_BRANCH_SHA=$(git ls-remote origin $CI_MERGE_REQUEST_TARGET_BRANCH_NAME | cut -d$'\t' -f1)
+FILES=$(git diff $CI_COMMIT_SHA $CI_MERGE_REQUEST_TARGET_BRANCH_SHA --name-only | tr '\n' ' ')
+echo "Running pre-commit on diff from $CI_COMMIT_SHA to $CI_MERGE_REQUEST_TARGET_BRANCH_SHA ($CI_MERGE_REQUEST_TARGET_BRANCH_NAME)"
+# Pass list of modified files to pre-commit so that it only checks them
+echo $FILES | xargs pre-commit run --color=always --files
diff --git a/requirements.txt b/requirements.txt
index ea6991b7d..d66d4c5d2 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,9 +1,10 @@
git+file:///gpfs/exfel/sw/calsoft/git/cal_db_interactive@2.0.1
git+file:///gpfs/exfel/sw/calsoft/git/nbparameterise@0.3
git+file:///gpfs/exfel/sw/calsoft/git/pyDetLib@2.5.6-2.10.0#subdirectory=lib
+Cython == 0.29.21
+Jinja2 == 2.11.2
astcheck == 0.2.5
astsearch == 0.1.3
-Cython == 0.29.21
dill == 0.3.0
extra_data == 1.2.0
extra_geom == 1.1.1
@@ -11,22 +12,22 @@ fabio == 0.9.0
gitpython == 3.1.0
h5py == 2.10.0
iminuit == 1.3.8
-ipyparallel == 6.2.4
ipykernel == 5.1.4
+ipyparallel == 6.2.4
ipython == 7.12.0
ipython_genutils == 0.2.0
-Jinja2 == 2.11.2
+jupyter-core == 4.6.1
jupyter_client == 6.1.7
jupyter_console == 6.1.0
-jupyter-core == 4.6.1
karabo_data == 0.7.0
lxml == 4.5.0
metadata_client == 3.0.8
nbclient == 0.5.1
nbconvert == 5.6.1
nbformat == 5.0.7
-notebook == 6.1.5
+notebook == 6.1.5
numpy == 1.19.1
+pre-commit == 2.10.0
prettytable == 0.7.2
princess == 0.2
pypandoc == 1.4
--
GitLab