Skip to content

Commit

Permalink
Fix #1288: Standardise mypy ignores and add dependencies README. (#1294)
Browse files Browse the repository at this point in the history
* Standardise mypy ignores.
* Add dependencies readme.
* Add flag to fail if pytest warnings present.
  • Loading branch information
calina-c authored Jun 26, 2024
1 parent 547997b commit 0a9aa4e
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 4 deletions.
73 changes: 73 additions & 0 deletions READMEs/dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<!--
Copyright 2023 Ocean Protocol Foundation
SPDX-License-Identifier: Apache-2.0
-->

# Dependency Management

This is a document for devs to understand how dependencies are managed in the `pdr-backend` project.
It also serves users who want to understand any issues and caveats in the installation process.

## Frozen Dependencies

- `pytest-asyncio` is frozen at version 0.21.1. This is because later versions have a known issue with event loops.

For more details, see the [the pytest-asyncio changelog](https://pytest-asyncio.readthedocs.io/en/latest/reference/changelog.html#id1), under Known Issues.
The library itself recommends using version 0.21 until the issue is resolved.

## For new dependencies: Types Management

For type checking, we use mypy, which may require dependencies for type hints.
This means that installing a new package would require installing additional packages for type hints.

In order to avoid conflicts with the main dependencies, we prefer not to install these additional packages.
Consequently, we ignore the missing library stubs in the `mypy.ini` file.

E.g. for pytz types if you see an error like:

```console
error: Library stubs not installed for "pytz" [import-untyped]
note: Hint: "python3 -m pip install types-pytz"
note: (or run "mypy --install-types" to install all missing stub packages)
note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
```

do NOT install the types-pytz package. Instead, add `pytz` to the `mypy.ini` file under the `[mypy]` section:

```ini
[mypy-pytz.*]
ignore_missing_imports = True
```

Use this approach for any other missing types, naming the section `[mypy-<package_name>].*`.

## For new dependencies/upgrade/removal: Check for warnings

When installing a new package, upgrading, or removing a package, check for warnings.
Currently the CIs fail if there are any warnings in the tests. This is to ensure that the codebase is clean and maintainable.

However, we do supress some errors in `pytest.ini`.

### "Permanent" suppressions: no need to check
- `ignore::pytest.PytestUnhandledThreadExceptionWarning`
- `ignore::pytest.PytestUnraisableExceptionWarning`
- `ignore:.*This process \(pid=.*\) is multi-threaded, use of fork\(\) may lead to deadlocks in the child.*:DeprecationWarning`

These are due to the nature of the pytest tests and are not expected to be resolved.

### "Temporary" suppressions: check and remove

#### Warnings related to selenium/dash-testing
- `ignore:.*HTTPResponse.getheader\(\) is deprecated.*` -> due to usage of getheader in selenium

If you upgrade selenium or dash[testing], you should check if these warnings are still present and remove the ignore statement in `pytest.ini` if they are not.

#### Warnings related to plotly
- `ignore:.*setDaemon\(\) is deprecated, set the daemon attribute instead.*:DeprecationWarning` -> due to usage of `kaleido` and `plotly`

If you upgrade plotly, you should check if these warnings are still present and remove the ignore statement in `pytest.ini` if they are not.

## Updating this document

If dependency upgrades fix some issues, or if policies change and this document needs to be updated, please do so.
Happy coding :)
3 changes: 3 additions & 0 deletions READMEs/dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,6 @@ pdr trader ppss.yaml sapphire-testnet
# or, run on mainnet
pdr trader ppss.yaml sapphire-mainnet
```

## Dependencies
See [dependencies.md](dependencies.md) for more details.
9 changes: 8 additions & 1 deletion mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ ignore_missing_imports = True
[mypy-pylab.*]
ignore_missing_imports = True

[mypy-pytest]
[mypy-pytest.*]
ignore_missing_imports = True

[mypy-pytz.*]
ignore_missing_imports = True

[mypy-requests.*]
Expand All @@ -85,3 +88,7 @@ ignore_missing_imports = True

[mypy-solcx.*]
ignore_missing_imports = True

[mypy-yaml.*]
ignore_missing_imports = True

2 changes: 2 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# pytest.ini
[pytest]
filterwarnings =
error
ignore::pytest.PytestUnhandledThreadExceptionWarning
ignore::pytest.PytestUnraisableExceptionWarning
ignore:.*HTTPResponse.getheader\(\) is deprecated.*
ignore:.*setDaemon\(\) is deprecated, set the daemon attribute instead.*:DeprecationWarning
ignore:.*This process \(pid=.*\) is multi-threaded, use of fork\(\) may lead to deadlocks in the child.*:DeprecationWarning
Expand Down
3 changes: 0 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
"pyarrow==16.1.0",
"pylint==3.2.3",
"pytest",
# pytest-asyncio: do not use dependabot to upgrade without checking lib changelog
"pytest-asyncio==0.21.1",
"pytest-env",
"pytz==2024.1",
Expand All @@ -45,8 +44,6 @@
"scikit-learn==1.5.0",
"statsmodels==0.14.2",
"typeguard==4.3.0",
"types-pytz==2024.1.0.20240417",
"types-pyYAML==6.0.12.20240311",
"xgboost==2.1.0",
"dash_bootstrap_components==1.6.0",
"web3==6.19.0",
Expand Down

0 comments on commit 0a9aa4e

Please sign in to comment.