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.

Sign up for GitHub

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

More CI (mypy, coverage), setup pre-commit, formatting #24

Merged
merged 14 commits into from
Mar 14, 2023
Merged

More CI (mypy, coverage), setup pre-commit, formatting #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7f56587
add pre-commit config file
benbovy Mar 14, 2023
3cf66eb
clang format tweaks
benbovy Mar 14, 2023
88386af
run black
benbovy Mar 14, 2023
55e39e7
ci: add mypy gha workflow
benbovy Mar 14, 2023
c6111d0
add pre-commit note in README
benbovy Mar 14, 2023
68d1432
fix mypy gha job
benbovy Mar 14, 2023
f6eb8ba
fix trailing whitespace
benbovy Mar 14, 2023
3c35bbd
configure mypy
benbovy Mar 14, 2023
ee2fdf3
use micromamba gha for tests
benbovy Mar 14, 2023
d55f7b5
fix tests gha
benbovy Mar 14, 2023
2dc00c9
typo
benbovy Mar 14, 2023
80a2fc7
enable coverage
benbovy Mar 14, 2023
2df03ab
try fix codecov upload
benbovy Mar 14, 2023
a99ba9d
add coverage badge in README
benbovy Mar 14, 2023
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
7 changes: 7 additions & 0 deletions .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
---
Language: Cpp
BasedOnStyle: Google
ColumnLimit: 100
TabWidth: 4
IndentWidth: 4
PointerAlignment: Left
ReferenceAlignment: Pointer
IndentAccessModifiers: false
AccessModifierOffset: -4
BinPackArguments: false
BinPackParameters: false
ExperimentalAutoDetectBinPacking: false
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortFunctionsOnASingleLine: Empty
AllowShortLambdasOnASingleLine: Inline
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have any opinion about how C++ code should look like, and how much we want to customize compared to the base Google style (so happy to follow your preferences). But just to consider, s2geography just follows google (https://github.com/paleolimbot/s2geography/blob/master/.clang-format), and arrow uses only minimal overrides (https://github.com/apache/arrow/blob/main/.clang-format)

(but so +1 on adding a linter for this to enforce consistent style!)

Copy link
Owner Author

@benbovy benbovy Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't have strong opinions either and I think it is a good idea to stay close to the Google style used by s2geography and not add too much customization. This is just to fix some minor things I find annoying, like packing some of the parameters on the same line when all the parameters don't fit on one line, e.g., I prefer this:

m.def("equals",
      py::vectorize(Predicate(s2geog::s2_equals)),
      py::arg("a"),
      py::arg("b"));

over this:

m.def("equals", py::vectorize(Predicate(s2geog::s2_equals)), py::arg("a"),
      py::arg("b"));

Copy link
Collaborator

@jorisvandenbossche jorisvandenbossche Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

As mentioned, I don't mind (as long as it is automated), so just follow your preferences ;)

...
39 changes: 39 additions & 0 deletions .github/workflows/lint.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
on:
push:
branches: [main]
pull_request:
branches: [main]

name: Lint

jobs:
lint:
name: mypy
runs-on: ubuntu-latest
defaults:
run:
shell: bash -l {0}

steps:
- name: Checkout repo
uses: actions/checkout@v3

- name: Setup micromamba
uses: mamba-org/provision-with-micromamba@v15
with:
environment-file: ci/environment.yml
environment-name: spherely-dev
extra-specs: |
python=3.11

- name: Build and install spherely
run: |
python -m pip install . -v --no-build-isolation

- name: Install mypy
run: |
python -m pip install 'mypy<0.990'

- name: Run mypy
run: |
python -m mypy --install-types --non-interactive
72 changes: 31 additions & 41 deletions .github/workflows/run-tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ jobs:
test:
name: ${{ matrix.os }}, ${{ matrix.python-version }}, ${{ matrix.env }}
runs-on: ${{ matrix.os }}
defaults:
run:
shell: bash -l {0}
strategy:
fail-fast: false
matrix:
Expand All @@ -36,41 +39,20 @@ jobs:
- name: Checkout repo
uses: actions/checkout@v3

- name: Setup mambaforge
uses: conda-incubator/setup-miniconda@v2
with:
miniforge-variant: Mambaforge
miniforge-version: latest
activate-environment: spherely-dev
use-mamba: true
python-version: ${{ matrix.python-version }}
environment-file: ${{ matrix.env }}

- name: Get Date
id: get-date
shell: bash -l {0}
# cache will last one day
run: echo "::set-output name=today::$(/bin/date -u '+%Y%m%d')"

- name: Cache environment
uses: actions/cache@v2
- name: Setup micromamba
uses: mamba-org/provision-with-micromamba@v15
with:
path: ${{ env.CONDA }}/envs
key: ${{ runner.os }}-{{ matrix.python-version }}-conda-${{ hashFiles( matrix.env ) }}-${{ steps.get-date.outputs.today }}-${{ env.CACHE_NUMBER }}
env:
# Increase this value to reset cache if ci/environment.yml has not changed
CACHE_NUMBER: 0
id: conda-cache

- name: Update environment
run: mamba env update -n spherely-dev -f ${{ matrix.env }}
if: steps.conda-cache.outputs.cache-hit != 'true'

- name: Conda info
shell: bash -l {0}
run: |
conda info
conda list
environment-file: ${{ matrix.env }}
environment-name: spherely-dev
cache-env: true
cache-env-key: "${{ runner.os }}-${{ runner.arch }}-py${{ matrix.python-version }}-${{ steps.get-date.outputs.today }}-${{ hashFiles( matrix.env) }}"
extra-specs: |
python=${{ matrix.python-version }}

- name: Fetch s2geography
uses: actions/checkout@v3
Expand All @@ -80,11 +62,9 @@ jobs:
path: deps/s2geography
fetch-depth: 0
if: |
matrix.dev == true &&
steps.conda-cache.outputs.cache-hit != 'true'
matrix.dev == true

- name: Configure, build & install s2geography (unix)
shell: bash -l {0}
run: |
cd deps/s2geography
cmake -S . -B build \
Expand All @@ -96,11 +76,9 @@ jobs:
cmake --install build
if: |
matrix.dev == true &&
(steps.conda-cache.outputs.cache-hit != 'true' &&
(runner.os == 'Linux' || runner.os == 'macOS'))
(runner.os == 'Linux' || runner.os == 'macOS')

- name: Configure, build & install s2geography (win)
shell: bash -l {0}
run: |
cd deps/s2geography
cmake -S . -B build \
Expand All @@ -111,15 +89,27 @@ jobs:
cmake --build build --config Release
cmake --install build
if: |
matrix.dev == true &&
(steps.conda-cache.outputs.cache-hit != 'true' &&
runner.os == 'Windows')
matrix.dev == true && runner.os == 'Windows'

- name: Build and install spherely
shell: bash -l {0}
run: python -m pip install . -v --no-build-isolation
run: |
python -m pip install . -v --no-build-isolation --config-settings cmake.define.SPHERELY_CODE_COVERAGE=ON --config-settings build-dir=_skbuild

- name: Run tests
shell: bash -l {0}
run: |
pytest . -vv

- name: Generate coverage report
run: |
python -m pip install gcovr
gcovr --exclude-unreachable-branches --print-summary -x -o coverage.xml
if: |
runner.os == 'Linux' && matrix.python-version == '3.11' && matrix.dev == false

- name: Upload coverage report
uses: codecov/codecov-action@v3
with:
files: ./coverage.xml
verbose: true
if: |
runner.os == 'Linux' && matrix.python-version == '3.11' && matrix.dev == false
22 changes: 22 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- id: debug-statements
- id: mixed-line-ending
- repo: https://github.com/psf/black
rev: 23.1.0
hooks:
- id: black
args: [--safe, --quiet]
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v15.0.7
hooks:
- id: clang-format
args: [--style=file]

ci:
autofix_prs: false
13 changes: 13 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ project(
set(CMAKE_CXX_STANDARD 17 CACHE STRING "The C++ standard to build with")
set(CMAKE_CXX_STANDARD_REQUIRED ON)

option(SPHERELY_CODE_COVERAGE "Enable coverage reporting" OFF)

# Dependencies

list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/third_party/cmake")
Expand All @@ -25,6 +27,11 @@ set(S2GEOGRAPHY_ROOT_DIR "$ENV{CONDA_PREFIX}")
find_package(s2 REQUIRED)
find_package(s2geography REQUIRED)

if(SPHERELY_CODE_COVERAGE)
message(STATUS "Building spherely with coverage enabled")
add_library(coverage_config INTERFACE)
endif()

# Compile definitions and flags

if (MSVC)
Expand Down Expand Up @@ -72,6 +79,12 @@ endif()

set_target_properties(spherely PROPERTIES CXX_VISIBILITY_PRESET "hidden")

if (SPHERELY_CODE_COVERAGE)
target_compile_options(coverage_config INTERFACE -O0 -g --coverage)
target_link_options(coverage_config INTERFACE --coverage)
target_link_libraries(spherely PUBLIC coverage_config)
endif()

# Install

install(TARGETS spherely LIBRARY DESTINATION .)
Expand Down
13 changes: 12 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

![Tests](https://github.com/benbovy/spherely/actions/workflows/run-tests.yaml/badge.svg)
[![Docs](https://readthedocs.org/projects/spherely/badge/?version=latest)](https://spherely.readthedocs.io)
[![Coverage](https://codecov.io/gh/benbovy/spherely/branch/main/graph/badge.svg)](https://app.codecov.io/gh/benbovy/spherely?branch=main)

*Python library for manipulation and analysis of geometric objects on the sphere.*

Expand Down Expand Up @@ -67,7 +68,17 @@ Run the tests:
$ pytest . -v
```

## Using the latest s2geography
Spherely also uses [pre-commit](https://pre-commit.com/) for code
auto-formatting and linting at every commit. After installing it, you can enable
pre-commit hooks with the following command:

```
$ pre-commit install
```

(Note: you can skip the pre-commit checks with `git commit --no-verify`)

## Using the latest s2geography version

If you want to compile spherely against the latest version of s2geography, use:

Expand Down
2 changes: 1 addition & 1 deletion docs/api.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,4 @@ Predicates
intersects
contains
within
disjoint
disjoint
5 changes: 5 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,8 @@ Repository = "https://github.com/benbovy/spherely"

[project.optional-dependencies]
test = ["pytest>=6.0"]

[tool.mypy]
files = ["tests", "src/spherely.pyi"]
show_error_codes = true
warn_unused_ignores = true
Loading