From 987a2fa211fbf960facca517262c76b343d82217 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 16:16:56 +0100 Subject: [PATCH 01/14] Add sphinx-design extension. --- environment.yml | 1 + setup.py | 1 + 2 files changed, 2 insertions(+) diff --git a/environment.yml b/environment.yml index 229de5e2..95dd946c 100644 --- a/environment.yml +++ b/environment.yml @@ -27,6 +27,7 @@ dependencies: - scikit-learn - sphinx - sphinx-automodapi + - sphinx-design - sphinx_rtd_theme - pydata-sphinx-theme - uproot~=4.1 diff --git a/setup.py b/setup.py index b283ec67..3ad62a26 100644 --- a/setup.py +++ b/setup.py @@ -29,6 +29,7 @@ "pydata_sphinx_theme", "numpydoc", "nbsphinx", + "sphinx-design", ] setup( From 93948c7851b5c08319882c9005a16dcd37174d17 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 16:17:07 +0100 Subject: [PATCH 02/14] Add CSS file. --- docs/_static/magicctapipe.css | 67 +++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 docs/_static/magicctapipe.css diff --git a/docs/_static/magicctapipe.css b/docs/_static/magicctapipe.css new file mode 100644 index 00000000..592bea83 --- /dev/null +++ b/docs/_static/magicctapipe.css @@ -0,0 +1,67 @@ +/* override table width restrictions */ +@media screen and (min-width: 767px) { + + .wy-table-responsive table td { + /* !important prevents the common CSS stylesheets from overriding + this as on RTD they are loaded after this stylesheet */ + white-space: normal !important; + } + + .wy-table-responsive { + overflow: visible !important; + } +} + +.version-switcher__container a[data-version-name*="stable"] { + background-color: #E9F6EC; + color: #28A745; +} + +.version-switcher__container a[data-version-name*="dev"] { + background-color: #FDF4EB; + color: #EE9040; +} + +html[data-theme="dark"] .version-switcher__container a[data-version-name*="stable"] { + background-color: #222924; + color: #28A745; +} + +html[data-theme="dark"] .version-switcher__container a[data-version-name*="dev"] { + background-color: #332A21; + color: #EE9040; +} + + +/* sphinx-design */ +.sd-card { + border-radius: 5px; + padding: 30px 10px 20px 10px; + margin: 10px 0px; +} + +.sd-card .sd-card-header .sd-card-text { + margin: 0px; +} + +.sd-card .sd-card-header { + border: none; + text-align: center; + font-size: var(--pst-font-size-h4); + font-weight: bold; + padding: 0.5rem 0rem 0.5rem 0rem; +} + +.sd-card .sd-card-footer { + border: none; +} + +.sd-card .sd-card-footer .sd-card-text { + max-width: 220px; + margin-left: auto; + margin-right: auto; +} + +html[data-theme="dark"] .sd-shadow-sm { + --sd-color-shadow: #6e6e6e; +} From 9bb7c9a16179fa10988fa0adc19dd63ed5d05848 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 16:17:20 +0100 Subject: [PATCH 03/14] Update sphinx configuration. --- docs/conf.py | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index f70ed4a7..ed175db8 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -13,6 +13,17 @@ setup_metadata = dict(setup_cfg.items("metadata")) setup_options = dict(setup_cfg.items("options")) +# -- General configuration + +extensions = [ + "sphinx.ext.duration", + "sphinx.ext.doctest", + "sphinx.ext.autodoc", + "sphinx.ext.autosummary", + "sphinx.ext.intersphinx", + "sphinx_design", +] + # -- Project information project = setup_metadata["name"] @@ -62,6 +73,7 @@ # We want to keep the relative reference when on a pull request or locally json_url = "_static/switcher.json" +# -- Options for HTML output # Theme options are theme-specific and customize the look and feel of a theme # further. For a list of options available for each theme, see the # documentation. @@ -100,17 +112,17 @@ "github_version": "master", "doc_path": "docs", } +html_css_files = ["magicctapipe.css"] +html_file_suffix = ".html" +# The name for this set of Sphinx documents. If None, it defaults to +# " v documentation". +html_title = f"{project} v{release}" -# -- General configuration +# Output file base name for HTML help builder. +htmlhelp_basename = project + "doc" -extensions = [ - "sphinx.ext.duration", - "sphinx.ext.doctest", - "sphinx.ext.autodoc", - "sphinx.ext.autosummary", - "sphinx.ext.intersphinx", -] +html_theme = "pydata_sphinx_theme" intersphinx_mapping = { "python": ("https://docs.python.org/3/", None), @@ -120,13 +132,5 @@ templates_path = ["_templates"] -# -- Options for HTML output - -html_theme = "pydata_sphinx_theme" - -# The name for this set of Sphinx documents. If None, it defaults to -# " v documentation". -html_title = f"{project} v{release}" - # -- Options for EPUB output epub_show_urls = "footnote" From b3c2ad6c4681076bbc6d661f5596ee93253e3a62 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 16:17:50 +0100 Subject: [PATCH 04/14] Add getting started. --- docs/{usage.rst => user-guide/getting-started.rst} | 6 ++++-- docs/user-guide/index.rst | 9 +++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) rename docs/{usage.rst => user-guide/getting-started.rst} (89%) create mode 100644 docs/user-guide/index.rst diff --git a/docs/usage.rst b/docs/user-guide/getting-started.rst similarity index 89% rename from docs/usage.rst rename to docs/user-guide/getting-started.rst index 4c3744b8..ee6ddab7 100644 --- a/docs/usage.rst +++ b/docs/user-guide/getting-started.rst @@ -1,5 +1,7 @@ -Usage -===== +.. _getting_started_users: + +Getting started for users +========================= .. _installation: diff --git a/docs/user-guide/index.rst b/docs/user-guide/index.rst new file mode 100644 index 00000000..81de7852 --- /dev/null +++ b/docs/user-guide/index.rst @@ -0,0 +1,9 @@ +.. _user-guide: + +User Guide +========== + +.. toctree:: + :maxdepth: 2 + + getting-started \ No newline at end of file From c8c73c9aae9c7cc796fbdae9b2b40a3857baa206 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 16:18:06 +0100 Subject: [PATCH 05/14] Update main index page. --- docs/index.rst | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index c1ef88e3..f9996583 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -15,9 +15,11 @@ Welcome to ``magic-cta-pipe`` documentation! **License**: BSD-3 +**Python**: |python_requires| + **magic-cta-pipe** is a pipeline for the analysis of joint data taken with MAGIC and LST-1. -Check out the :doc:`usage` section for the analysis steps, including how to :ref:`installation` the project. +Check out the :doc:`user-guide/index` section for the analysis steps, including how to :ref:`installation` the project. .. note:: @@ -26,6 +28,31 @@ Check out the :doc:`usage` section for the analysis steps, including how to :ref Contents -------- +.. _magicctapipe_docs: + .. toctree:: + :maxdepth: 1 + :hidden: + + user-guide/index + +.. grid:: 1 2 2 3 + + .. grid-item-card:: + + :octicon:`book;40px` + + User Guide + ^^^^^^^^^^ + + Learn how to get started as a user. This guide + will help you install and using magic-cta-pipe. + + +++ + + .. button-ref:: user-guide/index + :expand: + :color: primary + :click-parent: - usage \ No newline at end of file + To the user guide From a374f36ef743346927e231d8ce2141dcfb223752 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 16:31:34 +0100 Subject: [PATCH 06/14] Add section for MAGIC+LST analysis. --- docs/user-guide/index.rst | 3 +- docs/user-guide/magic-lst-scripts.rst | 45 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 docs/user-guide/magic-lst-scripts.rst diff --git a/docs/user-guide/index.rst b/docs/user-guide/index.rst index 81de7852..97d456d6 100644 --- a/docs/user-guide/index.rst +++ b/docs/user-guide/index.rst @@ -6,4 +6,5 @@ User Guide .. toctree:: :maxdepth: 2 - getting-started \ No newline at end of file + getting-started + magic-lst-scripts diff --git a/docs/user-guide/magic-lst-scripts.rst b/docs/user-guide/magic-lst-scripts.rst new file mode 100644 index 00000000..a126eace --- /dev/null +++ b/docs/user-guide/magic-lst-scripts.rst @@ -0,0 +1,45 @@ +.. _magic_lst_scripts: + +MAGIC+LST scripts +================= + +The scripts in the ``magicctapipe/scripts`` directory can be used to perform a MAGIC+LST analysis or a MAGIC-only analysis, as explained in the following. + +.. _magic_only_analysis: + +MAGIC-only analysis +------------------- + +MAGIC-only analysis starts from MAGIC-calibrated data (``_Y_`` files). The analysis flow is as follows: + +- ``magic_calib_to_dl1.py`` on real data, to convert them into DL1 format. If you use MC data produced with MMCS, you need to run this script over MC data as well. If you use SimTelArray MCs, run ``lst1_magic_mc_dl0_to_dl1.py`` over them instead. +- optionally, but recommended, ``merge_hdf_files.py`` to merge subruns and/or runs together +- ``lst1_magic_stereo_reco.py`` to add stereo parameters to the DL1 data (use ``--magic-only`` argument if the MC DL1 data contains LST-1 events) +- ``lst1_magic_train_rfs.py`` to train the RFs (energy, direction, classification) on train gamma MCs and protons +- ``lst1_magic_dl1_stereo_to_dl2.py`` to apply the RFs to stereo DL1 data (real and test MCs) and produce DL2 data +- ``lst1_magic_create_irf.py`` to create the IRF (use ``magic_stereo`` as ``irf_type`` in the configuration file) +- ``lst1_magic_dl2_to_dl3.py`` to create DL3 files, and ``create_dl3_index_files.py`` to create DL3 HDU and index files + +.. _magic_lst_analysis: + +MAGIC-LST analysis +------------------- + +MAGIC+LST analysis starts from MAGIC calibrated data (``_Y_`` files), LST DL1 data and SimTelArray DL0 data. The analysis flow is as following: + +- ``magic_calib_to_dl1.py`` on real MAGIC data, to convert them into DL1 format +- ``lst1_magic_mc_dl0_to_dl1.py`` over SimTelArray MCs to convert them into DL1 format +- optionally, but recommended, ``merge_hdf_files.py`` on MAGIC data to merge subruns and/or runs together +- ``lst1_magic_event_coincidence.py`` to find coincident events between MAGIC and LST-1, starting from DL1 data +- ``lst1_magic_stereo_reco.py`` to add stereo parameters to the DL1 data +- ``lst1_magic_train_rfs.py`` to train the RFs (energy, direction, classification) on train gamma MCs and protons +- ``lst1_magic_dl1_stereo_to_dl2.py`` to apply the RFs to stereo DL1 data (real and test MCs) and produce DL2 data +- ``lst1_magic_create_irf.py`` to create the IRF +- ``lst1_magic_dl2_to_dl3.py`` to create DL3 files, and ``create_dl3_index_files.py`` to create DL3 HDU and index files + +.. _high_level: + +High-level analysis +------------------- + +The folder ``notebooks`` contains Jupyter notebooks to perform checks on the IRF, to produce theta2 plots and SEDs. Note that the notebooks run with gammapy v0.20 or higher, therefore another conda environment is needed to run them, since the MAGIC+LST-1 pipeline at the moment depends on gammapy v0.19. From 34ad959ee37b5c4b2a0c97f2c4af57dbe62198dd Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 16:32:24 +0100 Subject: [PATCH 07/14] Remove README for magic-lst, moved to docs. --- magicctapipe/scripts/lst1_magic/README.md | 36 ----------------------- 1 file changed, 36 deletions(-) delete mode 100644 magicctapipe/scripts/lst1_magic/README.md diff --git a/magicctapipe/scripts/lst1_magic/README.md b/magicctapipe/scripts/lst1_magic/README.md deleted file mode 100644 index 6259c02f..00000000 --- a/magicctapipe/scripts/lst1_magic/README.md +++ /dev/null @@ -1,36 +0,0 @@ -# Script for MAGIC and MAGIC+LST analysis - -This folder contains scripts to perform MAGIC-only or MAGIC+LST analysis. - -Each script can be called from the command line from anywhere in your system (some console scripts are created during installation). Please run them with `-h` option for the first time to check what are the options available. - -## MAGIC-only analysis - -MAGIC-only analysis starts from MAGIC-calibrated data (\_Y\_ files). The analysis flow is as follows: - -- `magic_calib_to_dl1.py` on real and MC data (if you use MCs produced with MMCS), to convert them into DL1 format -- if you use SimTelArray MCs, run `lst1_magic_mc_dl0_to_dl1.py` over them to convert them into DL1 format -- optionally, but recommended, `merge_hdf_files.py` to merge subruns and/or runs together -- `lst1_magic_stereo_reco.py` to add stereo parameters to the DL1 data (use `--magic-only` argument if the MC DL1 data contains LST-1 events) -- `lst1_magic_train_rfs.py` to train the RFs (energy, direction, classification) on train gamma MCs and protons -- `lst1_magic_dl1_stereo_to_dl2.py` to apply the RFs to stereo DL1 data (real and test MCs) and produce DL2 data -- `lst1_magic_create_irf.py` to create the IRF (use `magic_stereo` as `irf_type` in the configuration file) -- `lst1_magic_dl2_to_dl3.py` to create DL3 files, and `create_dl3_index_files.py` to create DL3 HDU and index files - -## MAGIC+LST analysis: overview - -MAGIC+LST analysis starts from MAGIC calibrated data (\_Y\_ files), LST DL1 data and SimTelArray DL0 data. The analysis flow is as following: - -- `magic_calib_to_dl1.py` on real MAGIC data, to convert them into DL1 format -- `lst1_magic_mc_dl0_to_dl1.py` over SimTelArray MCs to convert them into DL1 format -- optionally, but recommended, `merge_hdf_files.py` on MAGIC data to merge subruns and/or runs together -- `lst1_magic_event_coincidence.py` to find coincident events between MAGIC and LST-1, starting from DL1 data -- `lst1_magic_stereo_reco.py` to add stereo parameters to the DL1 data -- `lst1_magic_train_rfs.py` to train the RFs (energy, direction, classification) on train gamma MCs and protons -- `lst1_magic_dl1_stereo_to_dl2.py` to apply the RFs to stereo DL1 data (real and test MCs) and produce DL2 data -- `lst1_magic_create_irf.py` to create the IRF -- `lst1_magic_dl2_to_dl3.py` to create DL3 files, and `create_dl3_index_files.py` to create DL3 HDU and index files - -## High level analysis - -The folder [Notebooks](https://github.com/cta-observatory/magic-cta-pipe/tree/master/notebooks) contains Jupyter notebooks to perform checks on the IRF, to produce theta2 plots and SEDs. Note that the notebooks run with gammapy v0.20 or higher, therefore another conda environment is needed to run them, since the MAGIC+LST-1 pipeline at the moment depends on v0.19. From 80fb827f19cceecdd2ae4c1660a7e2cc45fbeb25 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 18:16:03 +0100 Subject: [PATCH 08/14] Add index for developer guide. --- docs/developer-guide/index.rst | 13 +++++++++++++ docs/index.rst | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 docs/developer-guide/index.rst diff --git a/docs/developer-guide/index.rst b/docs/developer-guide/index.rst new file mode 100644 index 00000000..0c871615 --- /dev/null +++ b/docs/developer-guide/index.rst @@ -0,0 +1,13 @@ +.. _developer-guide: + +Developer Guide +=============== + +.. toctree:: + :maxdepth: 2 + + getting-started + style-guide + code-guidelines + pull-requests + maintainer-info diff --git a/docs/index.rst b/docs/index.rst index f9996583..82695a40 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -35,6 +35,7 @@ Contents :hidden: user-guide/index + developer-guide/index .. grid:: 1 2 2 3 @@ -56,3 +57,23 @@ Contents :click-parent: To the user guide + + .. grid-item-card:: + + :octicon:`git-branch;40px` + + Developer Guide + ^^^^^^^^^^^^^^^ + + Learn how to get started as a developer. + This guide will help you install magic-cta-pipe for development + and explains how to contribute. + + +++ + + .. button-ref:: developer-guide/index + :expand: + :color: primary + :click-parent: + + To the developer guide From 94f3c60e00c3af657795045c5bafeb8af688f045 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 18:16:18 +0100 Subject: [PATCH 09/14] Add getting started for developers. --- docs/user-guide/getting-started.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/user-guide/getting-started.rst b/docs/user-guide/getting-started.rst index ee6ddab7..c92b27b4 100644 --- a/docs/user-guide/getting-started.rst +++ b/docs/user-guide/getting-started.rst @@ -8,10 +8,10 @@ Getting started for users Installation ------------ -*magic-cta-pipe* and its dependencies may be installed using the *Anaconda* or *Miniconda* package system. We recommend creating a conda virtual environment +``magic-cta-pipe`` and its dependencies may be installed using the *Anaconda* or *Miniconda* package system. We recommend creating a conda virtual environment first, to isolate the installed version and dependencies from your master environment (this is optional). -The following command will set up a conda virtual environment, add the necessary package channels, and install *magic-cta-pipe* and its dependencies: +The following command will set up a conda virtual environment, add the necessary package channels, and install ``magic-cta-pipe`` and its dependencies: .. code-block:: console From c1298acb7754a7a692e9d6556e7cdcc0c69d5199 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 18:16:37 +0100 Subject: [PATCH 10/14] Add code and style guidelines. --- docs/developer-guide/code-guidelines.rst | 7 +++++++ docs/developer-guide/style-guide.rst | 4 ++++ 2 files changed, 11 insertions(+) create mode 100644 docs/developer-guide/code-guidelines.rst create mode 100644 docs/developer-guide/style-guide.rst diff --git a/docs/developer-guide/code-guidelines.rst b/docs/developer-guide/code-guidelines.rst new file mode 100644 index 00000000..853c6193 --- /dev/null +++ b/docs/developer-guide/code-guidelines.rst @@ -0,0 +1,7 @@ +.. _code_guidelines: + +Code Guidelines +=============== + +Coding should follow the CTA coding guidelines from the **CTA Code +Standards** document. See the `ctapipe code guidelines `_ for more information. \ No newline at end of file diff --git a/docs/developer-guide/style-guide.rst b/docs/developer-guide/style-guide.rst new file mode 100644 index 00000000..b04cea54 --- /dev/null +++ b/docs/developer-guide/style-guide.rst @@ -0,0 +1,4 @@ +Style Guide +================== + +``magic-cta-pipe`` follows the same style guide as ``ctapipe``. See the `ctapipe style guide `_ for more information. From 7016d154ce882f8c03633e690b75ff0815db84a9 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 18:17:03 +0100 Subject: [PATCH 11/14] Add getting started for developers (for real). --- docs/developer-guide/getting-started.rst | 438 +++++++++++++++++++++++ 1 file changed, 438 insertions(+) create mode 100644 docs/developer-guide/getting-started.rst diff --git a/docs/developer-guide/getting-started.rst b/docs/developer-guide/getting-started.rst new file mode 100644 index 00000000..2b88150e --- /dev/null +++ b/docs/developer-guide/getting-started.rst @@ -0,0 +1,438 @@ +.. _getting_started_dev: + +Getting Started For Developers +============================== + +.. warning:: + + The following guide is used only if you want to *develop* the + ``magic-cta-pipe`` package, if you just want to write code that uses it + as a dependency, you can install ``magic-cta-pipe`` from PyPI. + See :ref:`getting_started_users` + + +Forking vs. Working in the Main Repository +------------------------------------------ +If you are a member of CTA (Consortium or Observatory), or +otherwise a regular contributor to magic-cta-pipe, the maintainers can give you +access to the main repository at ``cta-observatory/magic-cta-pipe``. +Please contact Alessio Berti (alessioberti90@gmail.com) to get write access to the repository. +Working directly in the main repository has two main advantages +over using a personal fork: + +- No need for synchronisation between the fork and the main repository +- Easier collaboration on the same branch with other developers + +If you are an external contributor and don't plan to contribute regularly, +you need to go for the fork solution. + +The instructions below have versions for both approaches, select the tab that applies to your +setup. + + +Cloning the repository +---------------------- + +The examples below use ssh, assuming you setup an ssh key to access GitHub. +See `the GitHub documentation `_ if you haven't done so already. + +.. tab-set:: + + .. tab-item:: Working in the main repository + :sync: main + + Clone the repository: + + .. code-block:: console + + $ git clone git@github.com:cta-observatory/magic-cta-pipe.git + $ cd magic-cta-pipe + + + .. tab-item:: Working a fork + :sync: fork + + In order to checkout the software so that you can push changes to GitHub without + having write access to the main repository at ``cta-observatory/magic-cta-pipe``, you + `need to fork `_ it. + + After that, clone your fork of the repository and add the main reposiory as a second + remote called ``upstream``, so that you can keep your fork synchronized with the main repository. + + .. code-block:: console + + $ git clone https://github.com/[YOUR-GITHUB-USERNAME]/magic-cta-pipe.git + $ cd magic-cta-pipe + $ git remote add upstream https://github.com/cta-observatory/magic-cta-pipe.git + + + +Setting up the development environment +-------------------------------------- + +We provide a ``conda`` environment with all packages needed for development of ``magic-cta-pipe`` and a couple of additonal helpful pacakages (like ipython, jupyter and vitables): + +.. code-block:: console + + $ conda env create -f environment.yml + +This will install a ``conda`` environment called ``magic-lst``. Next, switch to this new virtual environment: + +.. code-block:: console + + $ conda activate magic-lst + +You will need to run that last command any time you open a new +terminal to activate the conda environment. + + +Installing magic-cta-pipe in development mode +--------------------------------------------- + +Now setup this cloned version for development. +The following command will use the editable installation feature of python packages. +From then on, all the magic-cta-pipe executables and the library itself will be +usable from anywhere, given you have activated the ``magic-lst`` conda environment. + +.. code-block:: console + + $ pip install -e . + +Using the editable installation means you will not have to rerun the installation for +simple code changes to take effect. +However, for things like adding new submodules or new entry points, rerunning the above +step might still be needed. + +We are using the ``black`` and ``isort`` auto-formatters for automatic +adherence to the code style (see our :doc:`/developer-guide/style-guide`). +To enforce running these tools whenever you make a commit, setup the +`pre-commit hook `_: + +.. code-block:: console + + $ pre-commit install + +The pre-commit hook will then execute the tools with the same settings as when the a pull request is checked on github, +and if any problems are reported the commit will be rejected. +You then have to fix the reported issues before tying to commit again. +Note that a common problem is code not complying with the style guide, and that whenever this was the only problem found, +simply adding the changes resulting from the pre-commit hook to the commit will result in your changes being accepted. + +Run the tests to make sure everything is OK: + +.. code-block:: console + + $ pytest + +Build the HTML docs locally and open them in your web browser, from the ``docs/_buid/html`` directory: + +.. code-block:: console + + $ make doc + +To update to the latest development version (merging in remote changes +to your local working copy): + + +.. tab-set:: + + .. tab-item:: Working in the main repository + :sync: main + + .. code-block:: console + + $ git pull + + .. tab-item:: Working a fork + :sync: fork + + .. code-block:: console + + $ git fetch upstream + $ git merge upstream/master --ff-only + $ git push + + Note: you can also press the "Sync fork" button on the main page of your fork on the github + and then just use ``git pull``. + +Developing a new feature or code change +--------------------------------------- + +You should always create a new branch when developing some new code. +Make a new branch for each new feature, so that you can make pull-requests +for each one separately and not mix code from each. +It is much easier to review and merge small, well-defined contributions than +a collection of multiple, unrelated changes. + +Most importantly, you should *never* add commits to the ``master`` branch of your fork, +as the ``master`` branch will often be updated in the main ``cta-observatory`` repository +and having a diverging history in the ``master`` branch of a fork will create issues when trying +to keep forks in sync. + +Remember that ``git switch `` [#switch]_ switches between branches, +``git switch -c `` creates a new branch and switches to it, +and ``git branch`` on it's own will tell you which branches are available +and which one you are currently on. + + +Create a feature branch +^^^^^^^^^^^^^^^^^^^^^^^ + +First think of a name for your code change, here we'll use +*implement_feature_1* as an example. + + +To ensure you are starting your work from an up-to-date ``master`` branch, +we recommend starting a new branch like this: + + +.. tab-set:: + + .. tab-item:: Working in the main repository + :sync: main + + .. code-block:: console + + $ git fetch # get the latest changes + $ git switch -c origin/master # start a new branch from master + + .. tab-item:: Working a fork + :sync: fork + + .. code-block:: console + + $ git fetch upstream # get latest changes from main repository + $ git switch -c upstream/master # start new branch from upstream/master + + + +Edit the code +^^^^^^^^^^^^^ + +and make as many commits as you want (more than one is generally +better for large changes!). + +.. code-block:: sh + + $ git add some_changed_file.py another_file.py + $ git commit + [type descriptive message in window that pops up] + +and repeat. The commit message should follow the *Git conventions*: +use the imperative, the first line is a short description, followed by a blank line, +followed by details if needed (in a bullet list if applicable). You +may even refer to GitHub issue ids, and they will be automatically +linked to the commit in the issue tracker. An example commit message:: + + fix bug #245 + + - changed the order of if statements to avoid logical error + - added unit test to check for regression + +Of course, make sure you frequently test via ``make test`` (or ``pytest`` in a +sub-module), check the style, and make sure the docs render correctly +(both code and top-level) using ``make doc``. + +.. note:: + + A git commit should ideally contain one and only one feature change + (e.g it should not mix changes that are logically different). + Therefore it's best to group related changes with ``git + add ``. You may even commit only *parts* of a changed file + using and ``git add -p``. If you want to keep your git commit + history clean, learn to use commands like ``git commit --ammend`` + (append to previous commit without creating a new one, e.g. when + you find a typo or something small). + + A clean history and a chain of well-written commit messages will + make it easier on code reviews to see what you did. + +Push your changes +^^^^^^^^^^^^^^^^^ + +The first time you push a new branch, you need to specify to which remote the branch +should be pushed [#push]_. Normally this will be ``origin``: + +.. code-block:: console + + $ git push -u origin implement_feature_1 + +After that first setup, you can push new changes using a simple + +.. code-block:: console + + $ git push + + +You can do this at any time and more than once. It just moves the changes +from your local branch on your development machine to your fork on github. + + +Integrating changes from the ``master`` branch. +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +In case of updates to the ``master`` branch during your development, +it might be necessary to update your branch to integrate those changes, +especially in case of conflicts. + +To get the latest changes, run: + +.. tab-set:: + + .. tab-item:: Working in the main repository + :sync: main + + .. code-block:: console + + $ git fetch + + .. tab-item:: Working a fork + :sync: fork + + .. code-block:: console + + $ git fetch upstream + +Then, update a local branch using: + +.. tab-set:: + + .. tab-item:: Working in the main repository + :sync: main + + .. code-block:: console + + $ git rebase origin/master + + or + + .. code-block:: console + + $ git merge origin/master + + .. tab-item:: Working a fork + :sync: fork + + .. code-block:: console + + $ git rebase upstream/master + + or + + .. code-block:: console + + $ git merge upstream/master + +For differences between rebasing and merging and when to use which, see `this tutorial `_. + + + +Create a *Pull Request* +^^^^^^^^^^^^^^^^^^^^^^^ + +When you're happy, you create PR on on your github fork page by clicking +"pull request". You can also do this via *GitHub Desktop* if you have +that installed, by pushing the pull-request button in the +upper-right-hand corner. + +Make sure to describe all the changes and give examples and use cases! + +See the :ref:`pull-requests` section for more info. + +Wait for a code review +^^^^^^^^^^^^^^^^^^^^^^ + +Keep in mind the following: + +* At least one reviewer must look at your code and accept your + request. They may ask for changes before accepting. +* All unit tests must pass. They are automatically run by *Travis* when + you submit or update your pull request and you can monitor the + results on the pull-request page. If there is a test that you added + that should *not* pass because the feature is not yet implemented, + you may `mark it as skipped temporarily + `_ until the + feature is complete. +* All documentation must build without errors. Again, this is checked + by *Travis*. It is your responsibility to run ``make doc`` and check + that you don't have any syntax errors in your docstrings. +* All code you have written should follow the style guide (e.g. no + warnings when you run the ``flake8`` syntax checker) + +If the reviewer asks for changes, all you need to do is make them, ``git +commit`` them and then run ``git push`` and the reviewer will see the changes. + +When the PR is accepted, the reviewer will merge your branch into the +*master* repo on cta-observatory's account. + +Delete your feature branch +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +since it is no longer needed (assuming it was accepted and merged in): + +.. code-block:: console + + $ git switch master # switch back to your master branch + +pull in the upstream changes, which should include your new features, and +remove the branch from the local and remote (github). + +.. tab-set:: + + .. tab-item:: Working in the main repository + :sync: main + + .. code-block:: console + + $ git pull + + .. tab-item:: Working a fork + :sync: fork + + .. code-block:: console + + $ git fetch upstream + $ git merge upstream/master --ff-only + +And then delete your branch: + +.. code-block:: console + + $ git branch --delete --remotes implement_feature_1 + + +Debugging Your Code +------------------- + +More often than not your tests will fail or your algorithm will +show strange behaviour. **Debugging** is one of the powerful tools each +developer should know. Since using ``print`` statements is **not** debugging and does +not give you access to runtime variables at the point where your code fails, we recommend +using ``pdb`` or ``ipdb`` for an IPython shell. +A nice introduction can be found `here `_. + +More Development help +--------------------- + +For coding details, read the :ref:`code_guidelines` section of this +documentation. + +To make git a bit easier (if you are on a Mac computer) you may want +to use the `github-desktop GUI `_, which +can do most of the fork/clone and remote git commands above +automatically. It provides a graphical view of your fork and the +upstream cta-observatory repository, so you can see easily what +version you are working on. It will handle the forking, syncing, and +even allow you to issue pull-requests. + +.. rubric:: Footnotes + +.. [#switch] ``git switch`` is a relatively new addition to git. If your version of git does not have it, update or use ``git checkout`` instead. The equivalent old command to ``git switch -c`` is ``git checkout -b``. + +.. [#push] As of git version 2.37, you can set these options so that ``git push`` will just work, + also for the first push: + + .. code-block:: console + + $ git config --global branch.autoSetupMerge simple + $ git config --global push.autoSetupRemote true \ No newline at end of file From b68d912e0826faa0ebd6c9ee95ddaf54d13c700c Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 18:17:17 +0100 Subject: [PATCH 12/14] Add pull requests management for developers. --- docs/developer-guide/pull-requests.rst | 98 ++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 docs/developer-guide/pull-requests.rst diff --git a/docs/developer-guide/pull-requests.rst b/docs/developer-guide/pull-requests.rst new file mode 100644 index 00000000..d073cd92 --- /dev/null +++ b/docs/developer-guide/pull-requests.rst @@ -0,0 +1,98 @@ +.. _pull-requests: + +Making and Accepting Pull Requests +================================== + +Making a Pull Request +--------------------- + +In the pull request description (editable on GitHub), you should +include the following information (some may be omitted if it is e.g. a +small bug fix and not a new feature or design change): + +* **What** does the change do? (description of the new features, changes, + or refactorings) +* **Where** should the reviewer start the review? (what is the central + module that changed, etc.) +* What is the **use case**, if it is a new feature? +* give an **example** of use or screenshot/plot if applicable +* Are any **new dependencies** required? (dependencies should be kept to a + minimum, so all new dependences need to be accepted by management) +* is there a relevant **issue** open that this addresses? (use the + #ISSUENUMBER syntax to link it) + + +Note that you can include syntax-highlighted code examples by using 3 back-tics: + +.. code-block:: none + + ```python + + code here + + ``` +.. warning:: + Note that the continous integration system will run only if you open a pull request + from a branch in the main repository. Pull requests opened from a branch from a + forked repository cannot run the continous integration system since they cannot + access the repository secrets used in some of the workflow (e.g. to download proprietary data). + +Continuous integration system +----------------------------- + +The *Travis* continuos integration (CI) system runs tests for pushes on a pull request +or the ``master`` branch. At the moment the CI system runs the following workflows: + +* ``release-drafter``, which drafts release notes (runs only for pushes on the ``master`` branch) +* ``lint``, which runs the ``pre-commit`` hook over the code +* ``pyflakes``, which runs ``pyflakes`` over the code +* ``docs``, which tests if the documentation can be built correctly +* ``tests``, which runs unit tests over a set of test files +* ``deploy``, which deploys automatically a new release to PyPI whenever a tag called ``v*`` + (e.g. ``v0.3.4``) is pushed to the ``master`` branch + +Keep in mind +------------ + +* make sure you remember to update the **documentation** as well as the code! + (see the ``docs/`` directory), and make sure it builds with no errors + (``make doc``) + +* pull requests that cause tests to fail on *Travis* will not be accepted until those tests pass + +.. * make sure to add a news fragment for the changelog. In order to do this add a file to the directory ``docs/changes`` and use the following naming scheme + ``..rst`` (take a look at the ``README`` inside of the directory for more details). The file should contain a brief summary of the purpose of this pull request. + + +Accepting a Pull Request +------------------------ + +``magic-cta-pipe`` maintainers must do a *code review* before accepting any +pull request. During the review the reviewer can ask for changes to be +made, and the requester can simply push them to the branch associated +with the request and they will automatically appear (no new pull +request needed). The following guidelines should be used to +facilitate the review procedure: + +* Perform a scientific or conceptual Review if the request introduces + new features, algorithms, or design changes + +* Look at the use case for the proposed change. + + - if the use case is missing, ask for one + - does it make sense? Is it connected to a goal, requirement, or specification? + +* Perform a Code Review + + - Check that all automatic checks succeeded, if not notify the author and give + guidance on how to fix the identified issues. + - Check that all functions and classes have API documentation in the + correct format. + - Check that there are at least basic unit tests for the added functionality / fixed bug. + - Check that the API (function and class definitions) is clear and + easy to understand. + - Check for common coding mistakes. + - Check for obvious performance issues. + - Check that the code uses the existing features of ctapipe. + - Check that the code doesn't introduce new features that are + already present in another form. From 6bd1d3efad2d6cf70cb15e76d0e0803a506037ca Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 18:17:31 +0100 Subject: [PATCH 13/14] Add maintainer info for developers. --- docs/developer-guide/maintainer-info.rst | 28 ++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/developer-guide/maintainer-info.rst diff --git a/docs/developer-guide/maintainer-info.rst b/docs/developer-guide/maintainer-info.rst new file mode 100644 index 00000000..b5946a91 --- /dev/null +++ b/docs/developer-guide/maintainer-info.rst @@ -0,0 +1,28 @@ +Maintainer info +=============== + +This is a collection of some notes for maintainers. + +How to update the online docs? +------------------------------ + +The docs are automatically built and deployed using ``readthedocs``. + + +How to make a release? +---------------------- +1. Open a new pull request to prepare the release. + This should be the last pull request to be merged before making the actual release. + + Add the planned new version to the ``docs/_static/switcher.json`` file, so it will be + available from the version dropdown once the documentation is built. + +2. After merging the pull request above, you should add a tag called ``v*`` (e.g. ``v0.3.4``) + in the ``master`` branch. In that way, the PyPI upload of the new release will be done + automatically by Github Actions. + +3. Create a new github release in the repository (https://github.com/cta-observatory/magic-cta-pipe/releases), + a good starting point should already be made by the release drafter plugin. + +.. 4. conda packages are built by conda-forge, the recipe is maintained here: https://github.com/conda-forge/ctapipe-feedstock/ + A pull request to update the recipe should be opened automatically by a conda-forge bot when a new version is published to PyPi. This can take a couple of hours. \ No newline at end of file From 812fe18fcc7458fe0b09845428c6c35d4e71cd89 Mon Sep 17 00:00:00 2001 From: Alessio Berti Date: Sun, 29 Oct 2023 18:17:48 +0100 Subject: [PATCH 14/14] Refer to development guide in main README. --- README.md | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 2af8d847..0aa07b84 100644 --- a/README.md +++ b/README.md @@ -29,29 +29,8 @@ In general, *magic-cta-pipe* is still in heavy development phase, so expect larg # Instructions for developers -People who would like to join the development of *magic-cta-pipe*, please contact Alessio Berti () to get write access to the repository. - -Developers should follow the coding style guidelines of the *ctapipe* project, see https://ctapipe.readthedocs.io/en/stable/developer-guide/style-guide.html and https://ctapipe.readthedocs.io/en/stable/developer-guide/code-guidelines.html. - -In short, to check for code/style errors and for reformatting the code: - -``` -pip install hacking # installs all checker tools -pip install black # installs black formatter -pyflakes magicctapipe # checks for code errors -flake8 magicctapipe # checks style and code errors -black filename.py # reformats filename.py with black -``` - -The *black* and *isort* auto-formatters are used for automatic adherence to the code style. To enforce running these tools whenever you make a commit, setup the [pre-commit hook](https://pre-commit.com/): - -```bash -$ pre-commit install -``` - -The pre-commit hook will then execute the tools with the same settings as when the a pull request is checked on github, and if any problems are reported the commit will be rejected. You then have to fix the reported issues before tying to commit again. - -In general, if you want to add a new feature or fix a bug, please open a new issue, and then create a new branch to develop the new feature or code the bug fix. You can create an early pull request even if it is not complete yet, you can tag it as "Draft" so that it will not be merged, and other developers can already check it and provide comments. When the code is ready, remove the tag "Draft" and select two people to review the pull request (at the moment the merge is not blocked if no review is performed, but that may change in the future). When the review is complete, the branch will be merged into the main branch. +Developers should follow the development install instructions found in the +[documentation](https://magic-cta-pipe.readthedocs.io/en/latest/developer-guide/getting-started.html).