lightly-ai / lightly-ai/lightly

Type Check Package

Open
#1,635 14 comments 1 reaction 0 assignees View on GitHub

A pull request for this has already been merged.

  • #1652 by @agarwal-ishaan — merged
good first issue help wanted
Dominant language
Python
Stars
3.8k
Forks
367
Avg merge
3d 22h
Merged PRs (30d)
5

Description

We would like to type check all files in the package. Adding types helps to document the code and makes it easier to use.

## How to work on this issue

1. Go to [pyproject.toml](https://github.com/lightly-ai/lightly/blob/master/pyproject.toml#L184) to the `exclude` section of the mypy settings. There should be a list of files like this:
```
exclude = '''(?x)(
lightly/cli/version_cli.py |
lightly/cli/crop_cli.py |
lightly/cli/serve_cli.py |
lightly/cli/embed_cli.py |
lightly/cli/lightly_cli.py |
...
```
2. Pick a file you would like to type. Best are files from `lightly/loss`, `lightly/data`, `lightly/utils`, `lightly/models/modules`, and the corresponding test files. Do not type check deprecated model files in `lightly/models` like `lightly/models/barlow_twins.py`.
3. Run `mypy ` (make sure that you have installed lightly following the [contribution guide](https://github.com/lightly-ai/lightly/blob/master/CONTRIBUTING.md)). This should show a list with all the missing types/errors.
4. Add types until no more errors are shown. This is a good example on how a typed file should look like: https://github.com/lightly-ai/lightly/blob/master/lightly/models/modules/memory_bank.py
5. Go to `pyproject.toml` and remove the filename of the file you just typed from the mypy `exclude` list.
6. If the file was the last file in a subdirectory that was missing types, then also remove the subdirectory name from the skip import list in [pyprojec.toml](https://github.com/lightly-ai/lightly/blob/master/pyproject.toml#L338-L346)
7. Create a new PR named `Add types for ` and push your changes. Make sure all Github actions pass.

**Important**
Lightly still supports old Python versions (including 3.7, 3.8, and 3.9) that do not work with new typing features introduced in Python 3.10. To use the new Python 3.10 syntax you have to add `from __future__ import annotations` at the top of the file. The following typing features changed in 3.10:
* Union types can now be written as `str | int` instead of `Union[str, int]`
* Optional types can now be written as `str | None` instead of `Optional[str]`
* Tuples, lists, and dicts can now be typed as `tuple[str, int]`, `list[str]`, `dict[str, int]` instead of `Tuple[str, int]`, `List[str]`, `Dict[str, int]`.

Please use `from __future__ import annotations` with the new types whenever possible. This will save a lot of refactoring in the future. Also feel free to update files with old types to the new syntax.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in pyproject.toml at the mypy exclude and skip-import sections, then choose a file from lightly/loss, lightly/data, lightly/utils, or lightly/models/modules. Run mypy on the selected file after following CONTRIBUTING.md, using lightly/models/modules/memory_bank.py as the typing example. Done means no mypy errors, the file is removed from the exclude list, relevant skip imports are updated, and GitHub Actions pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.