-
Notifications
You must be signed in to change notification settings - Fork 6
Standardized linting and formatting #39
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
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
8ecc4b4
Apply formatting
mjvogelsong 53a21f5
Add linter github action
mjvogelsong f44bd16
Add poetry.lock
mjvogelsong 407da2d
ignore import-error pylint
mjvogelsong 267ea68
Add pre-commit
mjvogelsong 4b78d37
Improve documentation
mjvogelsong 69aa0e6
Add more comments
mjvogelsong 41e30cd
Developer improvements
mjvogelsong 0081b99
Linter section of DEVELOPING.md
mjvogelsong 58b9079
Final cleanup
mjvogelsong 0e385b3
Improve makefile
mjvogelsong 30fc54a
Try fixing bot
mjvogelsong a156d75
Simplify
mjvogelsong 3627793
Merge branch 'main' into linting-improvements
mjvogelsong 7b5ee2c
Fix linter errors
mjvogelsong 8481d55
Update .pylintrc
mjvogelsong 31aa259
Merge branch 'main' into linting-improvements
mjvogelsong a87e7cd
Fix linter errors
mjvogelsong e6105a4
Formatter on md
mjvogelsong File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Run our linter on every push to the repository. | ||
| name: lint | ||
| on: [push] | ||
|
|
||
| jobs: | ||
| lint: | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| PYTHON_VERSION: "3.10" | ||
| POETRY_VERSION: "1.4.0" | ||
| steps: | ||
| - name: install python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: ${{ env.PYTHON_VERSION }} | ||
| - name: install poetry | ||
| uses: snok/install-poetry@v1 | ||
| with: | ||
| version: ${{ env.POETRY_VERSION }} | ||
| - name: get code | ||
| uses: actions/checkout@v3 | ||
| - name: install linter dependencies | ||
| run: | | ||
| make install-lint | ||
| - name: lint | ||
| run: | | ||
| make lint |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| # We call our local formatting script as a pre-commit hook. | ||
| # https://pre-commit.com/ | ||
| repos: | ||
| - repo: local | ||
| hooks: | ||
| - id: format | ||
| name: Format Code | ||
| entry: ./code-quality/format | ||
| language: script | ||
| types: [python, toml] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,8 @@ | ||
| { | ||
| "python.formatting.blackArgs": [ | ||
| "--line-length", | ||
| "120" | ||
| ], | ||
| "editor.formatOnSave": true, | ||
| "editor.codeActionsOnSave": { | ||
| "source.organizeImports": true, | ||
| }, | ||
| "python.analysis.extraPaths": [ | ||
| "./generated" | ||
| ], | ||
| "python.formatting.provider": "black", | ||
| } | ||
| "editor.formatOnSave": true, | ||
| "editor.codeActionsOnSave": { | ||
| "source.organizeImports": true | ||
| }, | ||
| "python.analysis.extraPaths": ["./generated"], | ||
| "python.formatting.provider": "black" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,45 +1,90 @@ | ||
| # Python SDK Developer Guide | ||
|
|
||
| The raw API is generated using an OpenAPI spec. The SDK adds commonly used functionality | ||
| like polling for blocking submits and configuration of tokens and endpoints. | ||
| The raw API is generated using an [OpenAPI spec](spec/public-api.yaml). The SDK adds commonly used | ||
| functionality like polling for blocking submits and configuration of tokens and endpoints. | ||
|
|
||
| ## Local Development | ||
|
|
||
| The auto-generated SDK code is in the `generated/` directory. To | ||
| re-generate the client code, you'll need to install | ||
| [openapi-generator](https://openapi-generator.tech/docs/installation#homebrew) | ||
| (I recommend homebrew if you're on a mac). Then you can run it with: | ||
| ### Install dependencies | ||
|
|
||
| ```Bash | ||
| $ make generate | ||
| First, make sure you have [poetry installed](https://python-poetry.org/docs/#installation). Then, | ||
| you can install the package dependencies by running: | ||
|
|
||
| ```shell | ||
| make install | ||
| ``` | ||
|
|
||
| ## Testing | ||
| Most tests need an API endpoint to run. This can be the public API endpoint `https://api.groundlight.ai`, | ||
| or a local endpoint with an edge client or development environment. | ||
| Note: We support Python 3.7+ for clients of the SDK, but we recommend developing with Python 3.10+. | ||
|
|
||
| ### Run tests | ||
|
|
||
| ### Getting the tests to use your current code. | ||
| Most tests need an API endpoint and an API token to run. The default endpoint is the public API | ||
| endpoint (`https://api.groundlight.ai`). But you can also test against other endpoints (like | ||
| localhost or integ). | ||
|
|
||
| (This needs to be updated.) | ||
| ```shell | ||
| # Run tests against the public API | ||
| GROUNDLIGHT_API_TOKEN="api_YOUR_PROD_TOKEN_HERE" make test | ||
| ``` | ||
|
|
||
| You kinda want to do a `pip install -e .` equivalent but I don't know how to do that with poetry. The ugly version is this... | ||
| But you can also test against other endpoints (like localhost or integ). Make sure the API token | ||
| comes from the same environment as the API endpoint you're testing against! | ||
|
|
||
| Find the directory where `groundlight` is installed: | ||
| ```shell | ||
| # Run tests against a local API | ||
| GROUNDLIGHT_API_TOKEN="api_YOUR_LOCALHOST_TOKEN_HERE" make test-local | ||
|
|
||
| # Run tests against the integ API | ||
| GROUNDLIGHT_API_TOKEN="api_YOUR_INTEG_TOKEN_HERE" make test-integ | ||
| ``` | ||
| $ python | ||
| Python 3.7.4 (default, Aug 13 2019, 20:35:49) | ||
| [GCC 7.3.0] :: Anaconda, Inc. on linux | ||
| Type "help", "copyright", "credits" or "license" for more information. | ||
| >>> import groundlight | ||
| >>> groundlight | ||
| <module 'groundlight' from '/home/ubuntu/anaconda3/lib/python3.7/site-packages/groundlight/__init__.py'> | ||
|
|
||
| ### Install auto-formatter pre-commit hook | ||
|
|
||
| We use [pre-commit](https://pre-commit.com/) to run some auto-formatters before committing code. You | ||
| can install the pre-commit hooks by running: | ||
|
|
||
| ```shell | ||
| make install-pre-commit | ||
| ``` | ||
|
|
||
| Then blow this away and set up a symlink from that directory to your source. | ||
| This will check to see whether any formatters need to be run every time you do `git commit`. If so, | ||
| it will run them, add the changes, and then ask you to try committing again with the new changes. | ||
|
|
||
| ### Generating code based on the latest API spec | ||
|
|
||
| The auto-generated SDK code is in the [generated/](generated) directory. Most of the time, you won't | ||
| need to generate code. But if the API specification changes, you may need to generate SDK code. To | ||
| re-generate the client code, you'll need to [install npm](https://github.com/nvm-sh/nvm#intro) | ||
| first. Then you can install the code generator by running: | ||
|
|
||
| ```shell | ||
| make install-generator | ||
| ``` | ||
| cd /home/ubuntu/anaconda3/lib/python3.7/site-packages/ | ||
| rm -rf groundlight | ||
| ln -s ~/dev/python-sdk/src/groundlight groundlight | ||
|
|
||
| Then you can generate the code by running: | ||
|
|
||
| ```shell | ||
| make generate | ||
| ``` | ||
|
|
||
| ### Linters | ||
|
|
||
| Linters help us find issues before runtime. We're currently using: | ||
|
|
||
| - [ruff](https://beta.ruff.rs/docs/) and | ||
| [pylint](https://pylint.readthedocs.io/en/latest/index.html) for general python linting. | ||
| - [mypy](https://mypy.readthedocs.io/en/stable/index.html) for type checking. | ||
| - [black](https://black.readthedocs.io/en/stable/index.html) for standardizing code formatting. | ||
| - [toml-sort](https://toml-sort.readthedocs.io/en/latest/) for linting the `pyproject.toml` file. | ||
|
|
||
| Most of these linters are configured in [pyproject.toml](pyproject.toml) (except for `pylint` in | ||
| [.pylintrc](.pylintrc)). Sometimes the linters are wrong or not useful, so we can add overrides. We | ||
| prefer to make the smallest possible override: | ||
|
|
||
| - single line override with a specific rule (e.g., `# pylint: disable=some-rule` or `# ruff: noqa: F403`) | ||
| - single file override with a specific rule | ||
| - single file override with a whole class of rules | ||
| - global override for a specific rule | ||
|
|
||
| Note: `pylint` is usually the strictest and slowest, so we may end up turning it off - but we'll try | ||
| it for a while and see how it goes! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,46 @@ | ||
| .PHONY: all test clean | ||
|
|
||
| install: ## Install the package from source | ||
| poetry install | ||
|
|
||
| generate: install ## Generate the SDK from our public openapi spec | ||
| install-lint: ## Only install the linter dependencies | ||
| poetry install --only lint | ||
|
|
||
| install-pre-commit: install ## Install pre-commit hooks | ||
| poetry run pre-commit install | ||
|
|
||
| install-generator: install ## Install dependencies for SDK code generator | ||
| npm install | ||
|
|
||
| generate: install-generator ## Generate the SDK from our public openapi spec | ||
| node_modules/.bin/openapi-generator-cli generate -i spec/public-api.yaml \ | ||
| -g python \ | ||
| -o ./generated | ||
| poetry run datamodel-codegen --input spec/public-api.yaml --output generated/model.py | ||
| poetry run black . | ||
|
|
||
| test-local: install ## Run integration tests against an API server running at http://localhost:8000/device-api (needs GROUNDLIGHT_API_TOKEN) | ||
| GROUNDLIGHT_ENDPOINT="http://localhost:8000/" poetry run pytest --cov=src test --log-cli-level INFO | ||
| PYTEST=poetry run pytest -v --cov=src | ||
|
|
||
| # You can pass extra arguments to pytest by setting the TEST_ARGS environment variable. | ||
| # For example: | ||
| # `make test TEST_ARGS="-k some_filter"` | ||
| TEST_ARGS= | ||
|
|
||
| test-integ: install ## Run integration tests against the integ API server (needs GROUNDLIGHT_API_TOKEN) | ||
| GROUNDLIGHT_ENDPOINT="https://api.integ.groundlight.ai/" poetry run pytest --cov=src test --log-cli-level INFO | ||
| test: install ## Run tests against the prod API (needs GROUNDLIGHT_API_TOKEN) | ||
| ${PYTEST} ${TEST_ARGS} test | ||
|
|
||
| test-local: install ## Run tests against a localhost API (needs GROUNDLIGHT_API_TOKEN and a local API server) | ||
| GROUNDLIGHT_ENDPOINT="http://localhost:8000/" ${PYTEST} ${TEST_ARGS} test | ||
|
|
||
| test-integ: install ## Run tests against the integ API server (needs GROUNDLIGHT_API_TOKEN) | ||
| GROUNDLIGHT_ENDPOINT="https://api.integ.groundlight.ai/" ${PYTEST} ${TEST_ARGS} test | ||
|
|
||
| test-docs: install ## Run the example code and tests in our docs against the prod API (needs GROUNDLIGHT_API_TOKEN) | ||
| poetry run pytest --markdown-docs docs -v | ||
| poetry run pytest -v --markdown-docs ${TEST_ARGS} docs | ||
|
|
||
| # Adjust which paths we lint | ||
| LINT_PATHS="src test bin samples" | ||
|
|
||
| lint: install-lint ## Run linter to check formatting and style | ||
| ./code-quality/lint ${LINT_PATHS} | ||
|
|
||
| format: install-lint ## Run standard python formatting | ||
| ./code-quality/format ${LINT_PATHS} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.