Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand the developer documentation #144

Merged
merged 16 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/source/community/developers/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ determine the **minimum** set of supported package versions:

In addition to this, the last 24 months of other dependencies should also be
supported.

## Axis ordering in spatial arrays

For arrays representing spatial data, we follow `zyx` axis ordering in the same way as `numpy` and `napari`. The origin is the upper left corner when you show the first element `stack[0, :, :]`. The first dimension is the one that you are slicing, the second is the height of the image, and the third is the width of the image. The [`brainglobe-space` package](/documentation/brainglobe-space/index.md) provides an interface to manipulate data following different conventions to adhere to this standard.
20 changes: 16 additions & 4 deletions docs/source/community/developers/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

There are many BrainGlobe repositories, so it may not be obvious where a new contribution should go.
If you're unsure about any part of the contributing process, please [get in touch](../../contact.md).

The best place for questions about contributing is probably
the [BrainGlobe Zulip chat](https://brainglobe.zulipchat.com/).
You are furthermore welcome to join the bi-weekly developer meetings and contribute items to the agenda - check out the [#developer-meeting stream on Zulip](https://brainglobe.zulipchat.com/#narrow/stream/414089-developer-meeting) (requires sign-in) for more information.
alessandrofelder marked this conversation as resolved.
Show resolved Hide resolved

If for any reason, you'd rather not reach out in public, feel free to send a direct message on Zulip
to [Adam Tyson](https://github.com/adamltyson), one of the core developers.
Expand All @@ -19,10 +21,19 @@ You can view these repositories and the relevant information by heading to the [

To add a new BrainGlobe atlas, please see the guide [here](/documentation/bg-atlasapi/adding-a-new-atlas).

## Creating a development environment
## To contribute code

Before contributing code, it may be useful to familiarise yourself with the [introduction to the BrainGlobe code for developers](./intro_to_codebase.md) as well as the [testing](./testing.md), [developer tooling](./tooling.md) and [conventions](./conventions.md) sections.

The core development team will support you in contributing code, irrespective of your experience.
At the same time, they will hold you to the same standards as they do each other on review, to ensure BrainGlobe remains easy-to-maintain.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this more friendly? I would suggest that we hold all contributions to the same standard, but not necessarily all developers (i.e. we may go in and fix something ourselves).

Suggested change
At the same time, they will hold you to the same standards as they do each other on review, to ensure BrainGlobe remains easy-to-maintain.
At the same time, they will hold you to the same standards as they do each other on review, to ensure BrainGlobe remains easy-to-maintain.



### Creating a development environment

It is recommended to use `conda` to install a development environment for
BrainGlobe projects. Once you have `conda` installed, the following commands
It is recommended to use a recent `conda` to install a development environment for
alessandrofelder marked this conversation as resolved.
Show resolved Hide resolved
BrainGlobe projects ([`conda` versions >=23.10.0](https://conda.org/blog/2023-11-06-conda-23-10-0-release/)
will significantly speed up installation time). Once you have `conda` installed, the following commands
will create and activate a `conda` environment with the requirements needed
for a development environment:

Expand Down Expand Up @@ -51,7 +62,7 @@ pip install -e '.[dev]'
from inside the repository. This will install the package, its dependencies,
and its development dependencies.

## Pull requests
### Pull requests

In all cases, please submit code to the main repository via a pull request. The developers recommend, and adhere,
to the following conventions:
Expand Down Expand Up @@ -98,5 +109,6 @@ conventions
testing
new_releases
specific_repos
intro_to_codebase
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be higher (first?) in the toctree? I would say:

  • Intro
  • Tooling
  • Conventions
  • Testing
  • Publishing
  • Specific repo
  • Code of conduct

Code of conduct <https://github.com/brainglobe/.github/blob/main/CODE_OF_CONDUCT.md>
:::
78 changes: 78 additions & 0 deletions docs/source/community/developers/intro_to_codebase.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Introduction to the BrainGlobe codebase for developers

This is an introduction to the high-level organisation of the BrainGlobe codebase for developers.
It serves as an introduction for new contributors and as a reference for all contributors.

It will cover BrainGlobe's guiding principles for development and the default software architecture for BrainGlobe repositories.
Note that this high-level organisation is (at least partially) aspirational, and implementing it is work-in-progress (see [the current roadmap](/community/roadmaps/index.md) and the [Core Development Project Board](https://github.com/orgs/brainglobe/projects/2))


## Guiding principles for development

The first guiding principle is that BrainGlobe should be easy-to-use by everyone.
In practice, this means that BrainGlobe code should be independent of species of interest or image modality, and should be installable and runnable by anyone with a reasonably modern laptop within minutes.
Users should not require programming expertise.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't strictly true. Can we specify something like "Users should be able to achieve their aims without programming experience"? We also strive to cater to programmers too, with APIs etc.


As a secondary guiding principle, we additionally aim to make the codebase **easy to maintain**, and **easy to contribute to**, where this doesn't interfere with the first principle.

Choices around the software architecture and technology stack (detailed below) are taken with these principles in mind.


### Examples of guiding principles in practice

* ease of installation: through the metapackage, we provide a one-line command to install all BrainGlobe tools at once. None of the packages depend on anything other than Python (we've removed historical compiled code), and are therefore easy to install cross-platform.
* accessibility: we aim to provide a Graphical User Interface (GUI) for all BrainGlobe tools. By asking users for feedback, we ensure that the GUI provides a nice user experience.
* ease of use through Python/interoperability: we aim to provide a well-documented Python API for all BrainGlobe tools.
* performance: By running weekly benchmarks comparing the latest release with the development version, we guarantee that performance will not deteriorate as BrainGlobe evolves.
* species/modality independent: none of the code makes any assumptions about the imaging modality or the species of the model organism of interest. We provide atlases for a variety of model organisms.
* useability: we sacrifice the code simplicity provided by `magicgui` in exchange for fine-grained control of the user experience by writing brainglobe widgets in `qtpy`. This is an example where the first guiding principle takes priority over the second.
* easy-to-maintain: we move functionality used by more than one independent BrainGlobe tool to `brainglobe-utils` to reduce code duplication and make maintenance easier.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason all of these bullets aren't capitalised?



## BrainGlobe Tools

Code providing functionality related to a specific analysis or visualisation step is referred to as a BrainGlobe "tool".
Each BrainGlobe tool has its own Github repository on the BrainGlobe organisation.

Currently stable tools are:
- [`brainglobe-atlasapi`](https://github.com/brainglobe/bg-atlasapi)
- [`brainglobe-heatmap`](https://github.com/brainglobe/brainglobe-heatmap)
- [`brainglobe-segmentation`](https://github.com/brainglobe/brainglobe-segmentation)
- [`brainreg`](https://github.com/brainglobe/brainreg)
- [`brainrender`](https://github.com/brainglobe/brainrender)
- [`cellfinder`](https://github.com/brainglobe/cellfinder)
- [`morphapi`](https://github.com/brainglobe/morphapi)

Tools currently in development are
- [`brainglobe-registration`](https://github.com/brainglobe/brainglobe-registration)
- [`brainrender-napari`](https://github.com/brainglobe/brainrender-napari)

The BrainGlobe Github organisation also hosts the [`brainglobe` (meta-)package](./repositories/brainglobe-meta/index.md) and the [`brainglobe-workflows` collection](./repositories/brainglobe-workflows/index.md) in separate repositories (which are not tools in themselves), as well as the utility packages `brainglobe-utils` and `brainglobe-space`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue that brainglobe-space is a tool. I've used it independently of the BG ecosystem before.


As can be seen from the package names, we follow a **loose** naming convention for packages following a pattern of {brain(globe) || cell}-{noun describing what the tool does}. The most important criterion is the expressiveness of the name.
The `bg-` prefix for BrainGlobe tools has been discontinued.

### User data

User data is kept in hidden folders, usually in the user's `$HOME` directory.
These folders are named after the tools (e.g. `~/.cellfinder/`) or in appropriate subfolders of `.brainglobe` (e.g. `~/.brainglobe/mpin_zfish_1um_v1.0/`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add that although this is the current status, all new locally stored files should go into ~.brainglobe?

Data that we provide (e.g. atlas data and test data) should be hosted on [GIN/BrainGlobe](https://gin.g-node.org/BrainGlobe/), and not on GitHub itself (unless it's a small package-specific text file, in which case it can go in [the package resources and accessed via `importlib`](https://docs.python.org/3/library/importlib.resources.html)).
We rely on [the `pooch` package](https://www.fatiando.org/pooch/latest/) to fetch data from GIN.

### Default architecture for BrainGlobe Tools

By default, each BrainGlobe tool should be organised into three distinct submodules: `core`, `qt`, and `napari`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe specify up to three? And we won't have this structure if there's no GUI.

These submodules should live in the same GitHub repository and are packaged together on PyPI.
It is acceptable to deviate from the default where there is a reason to.

* `core` contains the central logic to use this tool and exposes it through a Python API
* `qt` contains user-friendly widgets and related Qt code that is implemented using `qtpy`. Each of these widgets provides an intuitive graphical user interface for one part (or few related parts) of the Python API defined in `core`.
The widgets additionally emit Qt signals that any frontend (e.g. the code in `napari`) can connect to.
* `napari` contains at least one Napari plugin that connects to signals in `qt` and implements a napari-specific response (e.g. adding a Napari layer) to them.

The architecture has a number of advantages;
- the modularity ensure each of submodule can (and should be) tested individually in the first instance
- widgets can be re-used outside of napari
- integration tests in the same repo to avoid messy CI dependencies
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalisation?


Additionally, keeping the architecture consistent across BrainGlobe tools should make it easier to contribute to several tools once someone has contributed to one tool, due to the familiar codebase organisation.
14 changes: 14 additions & 0 deletions docs/source/community/developers/new_releases.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ Maintainers can trigger a new release by pushing a new tag, in the format `vX.Y.
The `v` prefix **is necessary** as the workflow will only attempt to upload to `PyPI` if the tag matches the format previously provided.
The `X`, `Y`, and `Z` values should be integers corresponding to the new version number.

## Coordinating releases with the documentation and the metapackage

Releases will be made ad-hoc as bug-fixes and new features become available.
When releasing a new version of a BrainGlobe repository, we also need to update the website, the metapackage, and any other tools that depend on that repository accordingly.
This means we will typically at least three dependent PRs;
alessandrofelder marked this conversation as resolved.
Show resolved Hide resolved
- One in the repository itself (containing the bugfix or new feature we'd like to release)
- One in the [website repository](https://github.com/brainglobe/brainglobe.github.io) (if necessary)
alessandrofelder marked this conversation as resolved.
Show resolved Hide resolved
- One for each repository that depends on the updated tool, to bump the dependency version.
alessandrofelder marked this conversation as resolved.
Show resolved Hide resolved
- One in the [metapackage repository](https://github.com/brainglobe/brainglobe-meta), which pins the new versions of all affected tools at once.
alessandrofelder marked this conversation as resolved.
Show resolved Hide resolved

We should cross-link the latter to the website update, and release all affected packages to PyPI (and conda if appropriate) once they are all merged into `main`.
Ideally, updates and releases should be made in an order that [follows the dependency tree](./repositories/brainglobe-meta/index#dependency-tree) - starting with our lower level tools, than their dependents, then dependents of those dependents, and so on.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link is giving me a warning. I don't think sphinx likes section references.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be happy if you put

./repositories/brainglobe-meta/index.md#dependency-tree

for the link - the .md gets auto-converted to .html so there aren't any problems there. I've used this syntax in a number of places in this repo already without any warnings showing up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worked around it differently, for now - but good to know :)

The meta-package itself will always be the last by this convention.

## Triggering a new release

The steps for triggering a new release are:
Expand Down
7 changes: 7 additions & 0 deletions docs/source/community/developers/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,10 @@ from PyPI to be set in repository secrets.
Many of these workflows use actions from
[neuroinformatics-unit/actions](https://github.com/neuroinformatics-unit/actions). Feel free to raise a PR to that
repository!

## Test data

Data used by the tests should be kept on [GIN](https://gin.g-node.org/BrainGlobe/) and fetched using `pooch`.
Test data should not live on GitHub.
To avoid local tests running checks on user data interfering with separate user data on the same machine, tests should mock test-user data by mocking `Path.home()` - an example of how to achieve this can [be viewed in `brainrender-napari`](https://github.com/brainglobe/brainrender-napari/blob/014f5c5908065ddaa5d6b05ecdf90493383cfa2f/tests/conftest.py).

5 changes: 4 additions & 1 deletion docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,7 @@
notfound_urls_prefix = None


linkcheck_ignore = ["https://neuromorpho.org/"]
linkcheck_ignore = [
"https://neuromorpho.org/",
"https://brainglobe.zulipchat.com/#narrow/stream/414089-developer-meeting"
]
Loading