{ "cells": [ { "cell_type": "markdown", "metadata": {}, "source": [ "# CLIMADA Development\n", "\n", "This is a guide about how to contribute to the development of CLIMADA. We first explain some general guidelines about when and how one can contribute to CLIMADA, and then describe the steps in detail. We assume that you are familiar with Git, Github and their commands. If you are not familiar with these, you can refer to our instructions for [Development with Git](Guide_Git_Development.ipynb). " ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "## Is CLIMADA the right place for your contribution? \n", "\n", "When developing for CLIMADA, it is important to distinguish between core content and particular applications. Core content is meant to be included into the [climada_python](https://github.com/CLIMADA-project/climada_python) repository and will be subject to a code review. Any new addition should first be discussed with one of the [repository admins](https://github.com/CLIMADA-project/climada_python/wiki/Developer-Board). The purpose of this discussion is to see\n", "\n", "- How does the planned module fit into CLIMADA?\n", "- What is an optimal architecture for the new module?\n", "- What parts might already exist in other parts of the code?\n", "\n", "Applications made with CLIMADA, such as an [ECA study](https://eca-network.org/) can be stored in the [paper repository](https://github.com/CLIMADA-project/climada_papers) once they have been published. For other types of work, consider making a separate repository that imports CLIMADA as an external package." ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "## Planning a new feature\n", "\n", "Here we're talking about large features such as new modules, new data sources, or big methodological changes. Any extension to CLIMADA that might affect other developers' work, modify the CLIMADA core, or need a big code review.\n", "\n", "Smaller feature branches don't need such formalities. Use your judgment, and if in doubt, let people know.\n", "\n", "### Talk to the group\n", " - Before starting coding a module, do not forget to coordinate with one of the repo admins (Emanuel, Chahan or Lukas)\n", " - This is the chance to work out the Big Picture stuff that is better when it's planned with the group - possible intersections with other projects, possible conflicts, changes to the CLIMADA core, additional dependencies\n", " - Also talk with others from the core development team ([see the GitHub wiki](https://github.com/CLIMADA-project/climada_python/wiki/Developer-Board)).\n", " - Bring it to a developers meeting - people may be able to help/advise and are always interested in hearing about new projects. You can also find reviewers!\n", " - Also, keep talking! Your plans _will_ change :)\n", "\n", "### Formulate the feature's data flow and workflow\n", "\n", "To optimize implementation and usefulness of the new feature, first conceptualize its data flow and workflow. It makes sense to discuss these with a CLIMADA core developer before starting to work on the feature's implementation.\n", "- **Data flow**: Outline of how data moves through the system — where it is created or input, how it is processed, and if and where it is stored. This helps to improve the computational efficiency and to identify potential bottlenecks. \n", "- **Workflow**: Plan about where and how the user and other CLIMADA components can interact with the new feature. This ensures that the new feature couples seamlessly to the existing code base of CLIMADA and that the new feaute is easily and clearly accessible to users.\n", "\n", "### Planning the work\n", "\n", "- Does the project go in its own repository and import CLIMADA, or does it extend the main CLIMADA repository. The way this is done is slowly changing, so definitely discuss it with the group.\n", "- Find a few people who will help to review your code.\n", " - Ask in a developers' meeting, on Slack (for WCR developers) or message people on the development team ([see the GitHub wiki](https://github.com/CLIMADA-project/climada_python/wiki/Developer-Board)).\n", " - Let them know roughly how much code will be in the reviews, and when you'll be creating pull requests.\n", "- How can the work split into manageable chunks?\n", " - A series of smaller pull requests is far more manageable than one big one (and takes off some of the pre-release pressure)\n", " - Reviewing and spotting issues/improvements/generalisations early is always a good thing.\n", " - It encourages modularisation of the code: smaller self-contained updates, with documentation and tests.\n", "- Will there be any changes to the CLIMADA core? These should be planned carefully\n", "- Will you need any new dependencies? Are you sure?" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "## Installing CLIMADA for development\n", "\n" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "To develop (or review a pull request), you need to setup a proper climada development environment. This is relatively easy but requires rigor, so please read all the instructions below and make sure to follow them (we also recommend to read everything once first, and then follow them from the start). \n", "\n", "First, follow the [Advanced instructions](../getting-started/install.rst#install-advanced). Note that if you want to work on a specific branch instead of `develop`, if you work on a feature for instance), you need to checkout that specific branc instead of `develop` after cloning:\n", "\n", "```\n", "git clone https://github.com/CLIMADA-project/climada_python.git\n", "cd climada_python\n", "git checkout \n", "```" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### Note on dependencies\n", "\n", "Climada dependencies are handled with the `requirements/env_climada.yml` file.\n", "When you run `mamba env update -n -f requirements/env_climada.yml`, the content of that file is used to install the dependencies, thus, if you are working on a branch that changes the dependencies, make sure to be on that branch **before** running the command." ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "## Working on feature branches\n", "\n", "When developing a big new feature, consider creating a feature branch and merging smaller branches into that feature branch with pull requests, keeping the whole process separate from `develop` until it's completed. This makes step-by-step code review nice and easy, and makes the final merge more easily tracked in the history.\n", "\n", "e.g. developing the big `feature/meteorite` module you might write `feature/meteorite-hazard` and merge it in, then `feature/meteorite-impact`, then `feature/meteorite-stochastic-events` etc... before finally merging `feature/meteorite` into `develop`. Each of these could be a reviewable pull request.\n", "\n", "### Make a new **branch**\n", "\n", "For new features in Git flow:\n", "\n", " git flow feature start feature_name\n", " \n", "Which is equivalent to (in vanilla git):\n", "\n", " git checkout -b feature/feature_name\n", "\n", "Or work on an existing branch:\n", "\n", " git checkout -b branch_name\n", "\n", "get the latest data from the remote repository and update your branch\n", " \n", " git pull\n", "\n", "Once you have set up everything (including pre-commit hooks) you will be able to:\n", "\n", "see your locally modified files\n", "\n", " git status\n", "\n", "add changes you want to include in the commit\n", "\n", " git add climada/modified_file.py climada/test/test_modified_file.py\n", "\n", "commit the changes\n", "\n", " git commit -m \"new functionality of .. implemented\"" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### Pre-Commit Hooks\n", "\n", "Climada developer dependencies include pre-commit hooks to help ensure code linting and formatting.\n", "See [Code Formatting](Guide_CLIMADA_conventions.ipynb#code-formatting) for our conventions regarding formatting.\n", "These hooks will run on all staged files and verify:\n", "\n", "- the absence of trailing whitespace\n", "- that files end in a newline and only a newline\n", "- the correct sorting of imports using ``isort``\n", "- the correct formatting of the code using ``black``\n", "\n", "If you have installed the pre-commit hooks (see [Install developer dependencies](../getting-started/install.rst#install-developer-dependencies-optional)), they will be run each time you attempt to create a new commit, and the usual git flow can slightly change:\n", "\n", "If any check fails, you will be warned and these hooks **will apply** corrections (such as formatting the code with black if it is not).\n", "As files are modified, you are required to stage them again (hooks cannot stage their modification, only you can) and commit again.\n", "\n", "As an exemple, suppose you made an improvement to Centroids and want to commit these changes, you would run:\n", "\n", "```console\n", "$ git status\n", "On branch feature/\n", "Your branch is up-to-date with 'origin/'.\n", "\n", "Changes to be committed:\n", " (use \"git restore --staged ...\" to unstage)\n", "\tmodified: climada/hazard/centroids/centr.py\n", "```\n", "\n", "Now trying to commit, and assuming that imports are not correctly sorted,\n", "and some of the code is not correctly formatted:\n", "\n", "```console \n", "$ git commit -m \"Add to centroids\"\n", "Fix End of Files.........................................................Passed\n", "Trim Trailing Whitespace.................................................Passed\n", "isort....................................................................Failed\n", "- hook id: isort\n", "- files were modified by this hook\n", "\n", "Fixing [...]/climada_python/climada/hazard/centroids/centr.py\n", "\n", "black-jupyter............................................................Failed\n", "- hook id: black-jupyter\n", "- files were modified by this hook\n", "\n", "reformatted climada/hazard/centroids/centr.py\n", "\n", "All done! ✨ 🍰 ✨\n", "```\n", "\n", "Note the commit was aborted, and the problems were fixed.\n", "However, these changes added by the hooks are not *staged* yet.\n", "You have to run ``git add`` again to stage them:\n", "\n", "```console\n", "$ git status\n", "On branch feature/\n", "Your branch is up-to-date with 'origin/'.\n", "\n", "Changes to be committed:\n", " (use \"git restore --staged ...\" to unstage)\n", "\tmodified: climada/hazard/centroids/centr.py\n", "\n", "Changes not staged for commit:\n", " (use \"git add ...\" to update what will be committed)\n", " (use \"git restore ...\" to discard changes in working directory)\n", "\tmodified: climada/hazard/centroids/centr.py\n", "\n", "$ git add climada/hazard/centroids/centr.py\n", "```\n", "\n", "After that, you can execute the commit and the hooks should pass:\n", "\n", "```console\n", "$ git commit -m \"Add to centroids\"\n", "Fix End of Files.........................................................Passed\n", "Trim Trailing Whitespace.................................................Passed\n", "isort....................................................................Passed\n", "black-jupyter............................................................Passed\n", "\n", "All done! ✨ 🍰 ✨\n", "```" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### Make unit and integration tests on your code, preferably during development\n", "\n", "Writing new code requires writing new tests: Please read our [Guide on unit and integration tests](Guide_Testing.ipynb)" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "## Pull requests\n", "\n", "We want every line of code that goes into the CLIMADA repository to be reviewed!\n", "\n", "Code review:\n", "- catches bugs (there are _always_ bugs)\n", "- lets you draw on the experience of the rest of the team\n", "- makes sure that more than one person knows how your code works\n", "- helps to unify and standardise CLIMADA's code, so new users find it easier to read and navigate\n", "- creates an archived description and discussion of the changes you've made\n", "\n", "### When to make a pull request\n", "\n", "- When you've finished writing a big new class or method (and its tests)\n", "- When you've fixed a bug or made an improvement you want to merge\n", "- When you want to merge a change of code into `develop` or `main`\n", "- When you want to _discuss_ a bit of code you've been working on - pull requests aren't only for merging branches\n", "\n", "Not all pull requests have to be into `develop` - you can make a pull request into any active branch that suits you.\n", "\n", "Pull requests need to be made latest two weeks before a release, see [releases](https://github.com/CLIMADA-project/climada_python/releases).\n", "\n", "### Step by step pull request!\n", "\n", "Let's suppose you've developed a cool new module on the `feature/meteorite` branch and you're ready to merge it into `develop`.\n", "\n", "### Checklist before you start\n", "\n", "- Documentation\n", "- Tests\n", "- Tutorial (if a complete new feature)\n", "- Updated dependencies (if need be)\n", "- Added your name to the AUTHORS file\n", "- Added an entry to the ``CHANGELOG.md`` file. See for information on how this shoud look like.\n", "- (Advanced, optional) interactively rebase/squash recent commits that _aren't yet on GitHub_.\n", "\n", "### Steps\n", "\n", "1) Make sure the `develop` branch is up to date on your own machine\n", " ```\n", " git checkout develop\n", " git pull\n", " ```\n", "\n", "2) Merge `develop` into your feature branch and resolve any conflicts\n", " ```\n", " git checkout feature/meteorite\n", " git merge develop\n", " ```\n", "\n", "In the case of more complex conflicts, you may want to speak with others who worked on the same code. Your IDE should have a tool for conflict resolution.\n", " \n", "3) Check all the tests pass locally\n", " ```\n", " make unit_test\n", " make integ_test\n", " ```\n", "\n", "4) Perform a static code analysis using pylint with CLIMADA's configuration `.pylintrc` (in the climada root directory). Jenkins executes it after every push.\\\n", " To do it locally, your IDE probably provides a tool, or you can run `make lint` and see the output in `pylint.log`.\n", "\n", "5) Push to GitHub.\n", " If you're pushing this branch for the first time, use\n", " ```\n", " git push -u origin feature/meteorite\n", " ```\n", " and if you're updating a branch that's already on GitHub:\n", " ```\n", " git push\n", " ```\n", "\n", "6) Check all the tests pass on the WCR Jenkins server (). See Emanuel's presentation for how to do this! You should regularly be pushing your code and checking this!\n", "\n", "7) Create the pull request!\n", "\n", " - On the CLIMADA GitHub page, navigate to your feature branch (there's a drop-down menu above the file structure, pointing by default to `main`).\n", " - Above the file structure is a branch summary and an icon to the right labelled \"Pull request\".\n", " - Choose which branch you want to merge with. This will usually be `develop`, but may be another feature branch for more complex feature development.\n", " - Give your pull request an informative title (like a commit message).\n", " - Write a description of the pull request. This can usually be adapted from your branch's commit messages (you wrote informative commit messages, didn't you?), and should give a high-level summary of the changes, specific points you want the reviewers' input on, and explanations for decisions you've made. The code documentation (and any references) should cover the more detailed stuff. \n", " - Assign reviewers in the page's right hand sidebar. Tag anyone who might be interested in reading the code. You should already have found one or two people who are happy to read the whole request and\n", " sign it off (they could also be added to 'Assignees').\n", " - Create the pull request.\n", " - Contact the reviewers to let them know the request is live. GitHub's settings mean that they may not be alerted automatically. Maybe also let people know on the WCR Slack!\n", "\n", "8) Talk with your reviewers\n", "\n", " - Use the comment/chat functionality within GitHub's pull requests - it's useful to have an archive of discussions and the decisions made.\n", " - Take comments and suggestions on board, but you don't need to agree with everything and you don't need to implement everything.\n", " - If you feel someone is asking for too many changes, prioritise, especially if you don't have time for complex rewrites.\n", " - If the suggested changes and or features don't block functionality and you don't have time to fix them, they can be moved to Issues.\n", " - Chase people up if they're slow. People are slow.\n", "\n", "\n", "9) Once you implement the requested changes, respond to the comments with the corresponding commit implementing each requested change.\n", "\n", "10) If the review takes a while, remember to merge `develop` back into the feature branch every now and again\n", " (and check the tests are still passing on Jenkins).\\\n", " Anything pushed to the branch is added to the pull request.\n", " \n", "11) Once everyone reviewing has said they're satisfied with the code you can merge the pull request using the GitHub interface.\\\n", " Delete the branch once it's merged, there's no reason to keep it. (Also try not to re-use that branch name later.)\n", " \n", "12) Update the `develop` branch on your local machine.\n", "\n", "Also see the [**Reviewer Guide**](Guide_Review.ipynb) and [**Reviewer Checklist**](Guide_Review.ipynb#reviewer-checklist)!" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "## General tips and tricks\n", "\n", "Follow the [python do's and don't](Guide_PythonDos-n-Donts) and [performance](Guide_Py_Performance.ipynb) guides. Write small readable methods, classes and functions.\n", "\n", "### Ask for help with Git\n", "\n", "- Git isn't intuitive, and rewinding or resetting is always work. If you're not certain what you're doing, or if you think you've messed up, send someone a message. See also our instructions for [Development with Git](Guide_Git_Development.ipynb).\n", "\n", "### Don't push or commit to develop or main\n", "\n", "- Almost all new additions to CLIMADA should be merged into the `develop` branch with a pull request.\n", "- You won't merge into the `main` branch, except for emergency hotfixes (which should be communicated to the team).\n", "- You won't merge into the `develop` branch without a pull request, except for small documentation updates and typos.\n", "- The above points mean you should never need to push the `main` or `develop` branches.\n", "\n", "So if you find yourself on the `main` or `develop` branches typing `git merge ...` or `git push` stop and think again - you should probably be making a pull request.\n", "\n", "This can be difficult to undo, so contact someone on the team if you're unsure!\n", "\n", "### Commit more often than you think, and use informative commit messages\n", "\n", "- Committing often makes mistakes less scary to undo\n", "```\n", "git reset --hard HEAD\n", "```\n", "- Detailed commit messages make writing pull requests really easy\n", "- Yes it's boring, but _trust me_, everyone (usually your future self) will love you when they're rooting through the git history to try and understand why something was changed\n", "\n", "### Commit message syntax guidelines\n", "\n", "Basic syntax guidelines taken from here (on 17.06.2020)\n", "\n", "- Limit the subject line to 50 characters\n", "- Capitalize the subject line\n", "- Do not end the subject line with a period\n", "- Use the imperative mood in the subject line (e.g. \"Add new tests\")\n", "- Wrap the body at 72 characters (most editors will do this automatically)\n", "- Use the body to explain what and why vs. how\n", "- Separate the subject from body with a blank line (This is best done with\n", " a GUI. With the command line you have to use text editor, you cannot\n", " do it directly with the git command)\n", "- Put the name of the function/class/module/file that was edited\n", "- When fixing an issue, add the reference gh-ISSUENUMBER to the commit message \n", " e.g. “fixes gh-40.” or “Closes gh-40.” For more infos see here .\n", "\n", "### What not to commit\n", "\n", "There are a lot of things that don't belong in the Git repository: \n", "- Don't commit data, except for config files and very small files for tests.\n", "- Don't commit anything containing passwords or authentication credentials or tokens. (These are annoying to remove from the Git history.) Contact the team if you need to manage authorisations within the code.\n", "- Don't commit anything that can be created by the CLIMADA code itself\n", "\n", "If files like this are going to be present for other users as well, add them to the repository's `.gitignore`.\n", "\n", "#### Jupyter Notebook metadata\n", "\n", "Git compares file versions by text tokens. Jupyter Notebooks typically contain a lot of metadata, along with binary data like image files. Simply re-running a notebook can change this metadata, which will be reported as file changes by Git. This causes excessive Diff reports that cannot be reviewed conveniently.\n", "\n", "To avoid committing changes of unrelated metadata, open Jupyter Notebooks in a text editor instead of your browser renderer. When committing changes, make sure that you indeed only commit things you *did* change, and revert any changes to metadata that are not related to your code updates.\n", "\n", "Several code editors use plugins to render Jupyter Notebooks. Here we collect the instructions to inspect Jupyter Notebooks as plain text when using them:\n", "- **VSCode**: Open the Jupyter Notebook. Then open the internal command prompt (`Ctrl` + `Shift` + `P` or `Cmd` + `Shift` + `P` on macOS) and type/select 'View: Reopen Editor with Text Editor'\n", "\n", "### Log ideas and bugs as GitHub Issues\n", "\n", "If there's a change you might want to see in the code - something that generalises, something that's not quite right, or a cool new feature - it can be set up as a GitHub Issue. Issues are pages for conversations about changes to the codebase and for logging bugs, and act as a 'backlog' for the CLIMADA project.\n", "\n", "For a bug, or a question about functionality, make a minimal working example, state which version of CLIMADA you are using, and post it with the Issue.\n", "\n", "### How not to mess up the timeline\n", "\n", "Git builds the repository through incremental edits. This means it's great at keeping track of its history. But there are a few commands that _edit_ this history, and if histories get out of sync on different copies of the repository you're going to have a bad time.\n", "\n", "- Don't rebase any commits that already exist remotely!\n", "- Don't `--force` anything that exists remotely unless you know what you're doing!\n", "- Otherwise, you're unlikely to do anything irreversible\n", "- You can do what you like with commits that only exist on your machine.\n", "\n", "That said, doing an interactive rebase to tidy up your commit history _before_ you push it to GitHub is a nice friendly gesture :)\n", "\n", "### Do not fast forward merges \n", "\n", "(This shouldn't be relevant - all your merges into `develop` should be through pull requests, which doesn't fast forward. But:)\n", "\n", "Don't fast forward your merges unless your branch is a single commit. Use\n", "`git merge --no-ff ...`\n", "\n", "The exceptions is when you're merging `develop` into your feature branch.\n", "\n", "### Merge the remote develop branch into your feature branch every now and again\n", "\n", "- This way you'll find conflicts early\n", "```\n", "git checkout develop\n", "git pull\n", "git checkout feature/myfeature\n", "git merge develop\n", "```\n", "\n", "### Create frequent pull requests\n", "\n", "I said this already:\n", "- It structures your workflow\n", "- It's easier for reviewers\n", "- If you're going to break something for other people you all know sooner\n", "- It saves work for the rest of the team right before a release\n", "\n", "### Whenever you do something with CLIMADA, make a new local branch \n", "\n", "You never know when a quick experiment will become something you want to save for later.\n", "\n", "### But do not do everything in the CLIMADA repository\n", "\n", "- If you're running CLIMADA rather than developing it, create a new folder, initialise a new repository with `git init` and store your scripts and data there\n", "- If you're writing an extension to CLIMADA that doesn't change the model core, create a new folder, initialise a new repository with `git init` and import CLIMADA. You can always add it to the model later if you need to.\n", "\n", "### Questions\n", "\n", "![Git and Github logos](img/xkcd_git.png)\\\n", "" ] } ], "metadata": { "kernelspec": { "display_name": "", "name": "" }, "language_info": { "name": "python" } }, "nbformat": 4, "nbformat_minor": 4 }