-
Notifications
You must be signed in to change notification settings - Fork 72
use literalinclude in code-style-linting-format #259
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,40 @@ | ||||||
repos: | ||||||
- repo: https://github.com/PyCQA/isort | ||||||
rev: 5.11.4 | ||||||
hooks: | ||||||
- id: isort | ||||||
files: \.py$ | ||||||
|
||||||
# Misc commit checks using built in pre-commit checks | ||||||
- repo: https://github.com/pre-commit/pre-commit-hooks | ||||||
rev: v4.4.0 | ||||||
# ref: https://github.com/pre-commit/pre-commit-hooks#hooks-available | ||||||
hooks: | ||||||
# Autoformat: Makes sure files end in a newline and only a newline. | ||||||
- id: end-of-file-fixer | ||||||
|
||||||
# Lint: Check for files with names that would conflict on a | ||||||
# case-insensitive filesystem like MacOS HFS+ or Windows FAT. | ||||||
- id: check-case-conflict | ||||||
- id: trailing-whitespace | ||||||
|
||||||
# Linting: Python code (see the file .flake8) | ||||||
- repo: https://github.com/PyCQA/flake8 | ||||||
rev: "6.0.0" | ||||||
hooks: | ||||||
- id: flake8 | ||||||
|
||||||
# Black for auto code formatting | ||||||
repos: | ||||||
- repo: https://github.com/psf/black | ||||||
rev: 22.12.0 | ||||||
hooks: | ||||||
- id: black | ||||||
language_version: python3.8 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just porting the examples as they were originally written. |
||||||
|
||||||
# Tell precommit.ci bot to update codoe format tools listed in the file | ||||||
# versions every quarter | ||||||
# The default it so update weekly which is too many new pr's for many | ||||||
# maintainers (remove these lines if you aren't using the bot!) | ||||||
ci: | ||||||
autoupdate_schedule: quarterly |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,3 +36,14 @@ docs = [ | |
"sphinx", | ||
"pydata-sphinx-theme" | ||
] | ||
|
||
[tool.ruff] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh.. i may be confused. here you do have ruff. please help me understand this config! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the guide originally had examples for both black and ruff, in different sections, as it covers both. There is nothing wrong about having a pyproject.toml with both, the project will work just fine (although yes, confusing) even if the tools are set up to use conflicting settings. The |
||
select = [ | ||
"E", # pycodestyle errors | ||
"W", # pycodestyle warnings | ||
"F", # pyflakes. "E" + "W" + "F" + "C90" (mccabe complexity) is equivalent to flake8 | ||
"I", # isort | ||
] | ||
|
||
[tool.ruff.isort] | ||
known-first-party = ["examplePy"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
def celsius_to_fahrenheit(celsius): | ||
""" | ||
Convert temperature from Celsius to Fahrenheit. | ||
|
||
Parameters: | ||
celsius (float): Temperature in Celsius. | ||
|
||
Returns: | ||
float: Temperature in Fahrenheit. | ||
Comment on lines
+5
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we prefer to use numpy style docstrings and suggest that in our guidebook. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jeremy, i think i understand what you are doing - in terms of having things in "other" formats. so you can likely ignore ALL of my inline suggestions -- BUT i don't understand how it's implemented in the guidebook. if you can give me instructions on how this would be run or how someone interacts with it that would be great!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was not trying to be different, again I just ported the examples as they already existed. |
||
""" | ||
fahrenheit = (celsius * 9/5) + 32 | ||
return fahrenheit | ||
|
||
|
||
def fahrenheit_to_celsius(fahrenheit): | ||
""" | ||
Convert temperature from Fahrenheit to Celsius. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use numpy style here and throughout! thank you! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here are you showing other docstring formats? will those be used elsewhere in our guidebook? |
||
|
||
Parameters: | ||
fahrenheit (float): Temperature in Fahrenheit. | ||
|
||
Returns: | ||
float: Temperature in Celsius. | ||
""" | ||
celsius = (fahrenheit - 32) * 5/9 | ||
return celsius |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,15 @@ | ||||||
from examplePy.temperature import fahrenheit_to_celsius | ||||||
import pandas | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
from typing import Sequence | ||||||
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if we follow isort / pep8 we'd want examplePy to be the last import? or were you doing this intentionally to allow isort to take action is a user tries to run it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did this intentionally in order to make a counter example. Notice that this module is named |
||||||
|
||||||
def calc_annual_mean(df: pandas.DataFrame): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
or just import DataFrame from pandas above? |
||||||
"""Function to calculate the mean temperature for each year and the final mean""" | ||||||
# TODO: make this a bit more robust so we can write integration test examples?? | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a comment from me? i kind of recall talking about tests but also we could potentially remove the todo here in the example package? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is from you 😄 ec0255c4 |
||||||
# Calculate the mean temperature for each year | ||||||
yearly_means = df.groupby('Year').mean() | ||||||
|
||||||
# Calculate the final mean temperature across all years | ||||||
final_mean = yearly_means.mean() | ||||||
|
||||||
# Return a converted value | ||||||
return fahrenheit_to_celsius(yearly_means), fahrenheit_to_celsius(final_mean) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
from typing import Sequence | ||
|
||
import pandas | ||
|
||
from examplePy.temperature import fahrenheit_to_celsius | ||
|
||
def calc_annual_mean(df: pandas.DataFrame): | ||
"""Function to calculate the mean temperature for each year and the final mean""" | ||
# TODO: make this a bit more robust so we can write integration test examples?? | ||
# Calculate the mean temperature for each year | ||
yearly_means = df.groupby('Year').mean() | ||
|
||
# Calculate the final mean temperature across all years | ||
final_mean = yearly_means.mean() | ||
|
||
# Return a converted value | ||
return fahrenheit_to_celsius(yearly_means), fahrenheit_to_celsius(final_mean) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,8 +38,8 @@ a discussion of: | |
## Use a code format tool (or tools) to make your life easier | ||
|
||
We suggest that you use a code format tool, or a set of format tools, because | ||
manually applying all of the PEP 8 format specifications is time consuming | ||
for both maintainers and can be a road block for potential new contributors. | ||
manually applying all of the PEP 8 format specifications is both time consuming | ||
for maintainers and can be a road block for potential new contributors. | ||
Code formatters will automagically reformat your code for you, adhering to | ||
PEP 8 standards and applying consistent style decisions throughout. | ||
|
||
|
@@ -70,29 +70,29 @@ discussed below, is an example of a commonly-used code linter. | |
|
||
### Code Formatters (and stylers) | ||
|
||
Python focused code formatters often follow PEP 8 standards. However, they also | ||
make stylistic decisions about code consistency. Code formatters will | ||
reformat your code for you. | ||
Code formatters will reformat your code for you. Python focused code formatters | ||
often follow PEP 8 standards. However, they also make stylistic decisions about | ||
code consistency. | ||
|
||
Black is an example of a commonly-used code formatter. Black both applies PEP 8 | ||
standards while also making decisions about things like consistent use of double | ||
quotes for strings, and spacing of items in lists. | ||
|
||
You will learn more about Black below. | ||
|
||
## Code format and style | ||
## Code linting, formatting and styling tools | ||
|
||
### Black | ||
|
||
[Black](https://black.readthedocs.io/en/stable/) is a code | ||
formatter. Black will automagically (and _unapologetically_) | ||
fix spacing issues and ensure code format is consistent throughout your | ||
package. Black also generally adhere to PEP 8 style guidelines with | ||
package. Black also generally adheres to PEP 8 style guidelines with | ||
some exceptions. A few examples of those exceptions are below: | ||
|
||
- Black defaults to a line length of 88 (79 + 10%) rather than the 79 character `PEP 8` specification. However, line length is a setting can be manually overwritten in your Black configuration. | ||
- Black will not adjust line length in your comments or docstrings. | ||
- This tool will not review and fix import order (you need _isort_ or _Ruff_ to do that - see below). | ||
- This tool will not review and fix import order (you need `isort` or `ruff` to do that - see below). | ||
|
||
```{tip} | ||
If you are interested in seeing how Black will format your code, you can | ||
|
@@ -102,7 +102,7 @@ use the [Black playground](https://black.vercel.app/) | |
Using a code formatter like Black will leave you more time to work on | ||
code function rather than worry about format. | ||
|
||
### flake8 for linting code in Python packages | ||
### Flake8 | ||
|
||
To adhere to Python `pep8` format standards, you might want to add | ||
[flake8](https://flake8.pycqa.org/en/latest/) to your code format | ||
|
@@ -122,9 +122,8 @@ called `stravalib`. | |
|
||
The line length standard for PEP 8 is 79 characters. | ||
|
||
Notice that | ||
flake8 returns a list of issues that it found in the model.py module in the | ||
command line. The Python file itself is not modified. Using on this output, | ||
Notice that flake8 returns a list of issues that it found in the model.py module | ||
on the command line. The Python file itself is not modified. Using this output, | ||
you can fix each issue line by line manually. | ||
|
||
```bash | ||
|
@@ -152,7 +151,7 @@ your package. | |
> - Related third party imports. | ||
> - Local application/library specific imports. | ||
|
||
While **flake8** will identify unused imports in your code, it won't | ||
While `flake8` will identify unused imports in your code, it won't | ||
fix or identify issues with the order of package imports. | ||
|
||
`isort` will identify where imports in your code are out of | ||
|
@@ -164,75 +163,57 @@ up your code. | |
|
||
Code imports before `isort` is run: | ||
|
||
Below, the exc module is a part of starvalib which is a | ||
third party package. `abc` and `logging` are core `Python` packages | ||
distributed with `Python`. Also notice that there are extra | ||
spaces in the imports listed below. | ||
|
||
```python | ||
from stravalib import exc | ||
import abc | ||
import logging | ||
|
||
from collections.abc import Sequence | ||
Below, the `pandas` is a third party package, `typing` is a core `Python` | ||
package distributed with `Python`, and `examplePy.temperature` is a first-party | ||
module which means it belongs to the same package as the file doing the import. | ||
Also notice that there are no spaces in the imports listed below. | ||
|
||
:::{literalinclude} ../examples/pure-hatch/src/examplePy/temporal-raw.py | ||
:language: python | ||
:end-before: def calc_annual_mean | ||
::: | ||
|
||
from stravalib import unithelper as uh | ||
From the project root, run: | ||
```bash | ||
isort src/examplePy/temporal.py | ||
``` | ||
|
||
Run: | ||
`isort stravalib/model.py` | ||
Python file `temporal.py` imports after `isort` has been run | ||
|
||
Python file model.py imports after `isort` has been run | ||
|
||
```python | ||
import abc | ||
import logging | ||
from collections.abc import Sequence | ||
|
||
from stravalib import exc | ||
from stravalib import unithelper as uh | ||
``` | ||
:::{literalinclude} ../examples/pure-hatch/src/examplePy/temporal.py | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahhhh i think this answers my question above about the imports. i haven't run this locally. maybe you can help me understand what will happen here in our guidebook with code examples that have formatting issues. will isort run? or would we tell the user to have it run as a small demo that they could implement? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nothing runs, |
||
:language: python | ||
:end-before: def calc_annual_mean | ||
::: | ||
|
||
### Ruff | ||
|
||
[Ruff](https://beta.ruff.rs) is a new addition to the code quality ecosystem, | ||
gaining some traction since its release. | ||
`ruff` is a linter for Python, aiming to replace several tools behind a single interface. | ||
As such, `ruff` can be used instead of `flake8` and `isort`. | ||
[Ruff](https://docs.astral.sh/ruff/) is a new addition to the code quality | ||
ecosystem, gaining some traction since its release. `ruff` is both a linter | ||
and a code formatter for Python, aiming to replace several tools behind a single | ||
interface. As such, `ruff` can be used at a replacement of all other tools | ||
mentioned here, or in complement to some of them. | ||
|
||
`ruff` has some interesting features that distinguish it from other linters: | ||
|
||
- Linter configuration in `pyproject.toml` | ||
- Several hundred rules included, many of which are automatically fixable | ||
- Rules explanation, see [F403](https://beta.ruff.rs/docs/rules/undefined-local-with-import-star/) for an example | ||
- Rules explanation, see [F403](https://docs.astral.sh/ruff/rules/undefined-local-with-import-star/) for an example | ||
- Fast execution time, makes a quick feedback loop possible even on large projects. | ||
|
||
Here is a simple configuration to get started with `ruff`: | ||
|
||
```toml | ||
# pyproject.toml | ||
|
||
[tool.ruff] | ||
select = [ | ||
"E", # pycodestyle errors | ||
"W", # pycodestyle warnings | ||
"F", # pyflakes. "E" + "W" + "F" + "C90" (mccabe complexity) is equivalent to flake8 | ||
"I", # isort | ||
] | ||
ignore = [ | ||
"E501", # line >79, handled by black | ||
] | ||
``` | ||
Here is a simple configuration to get started with `ruff`. It would go into your `pyproject.toml`: | ||
|
||
Depending on your project, you might want to add the following to sort imports correctly: | ||
:::{literalinclude} ../examples/pure-hatch/pyproject.toml | ||
:language: toml | ||
:start-at: [tool.ruff] | ||
:end-before: [tool.ruff.isort] | ||
::: | ||
|
||
```toml | ||
# pyproject.toml | ||
Depending on your project, you might want to add the following to sort imports correctly: | ||
|
||
[tool.ruff.isort] | ||
known-first-party = ["<package_folder>"] | ||
``` | ||
:::{literalinclude} ../examples/pure-hatch/pyproject.toml | ||
:language: toml | ||
:start-at: [tool.ruff.isort] | ||
::: | ||
|
||
## How to use code formatter in your local workflow | ||
|
||
|
@@ -347,50 +328,9 @@ Below is an example **.pre-commit-cofig.yaml** file that can be used to setup | |
the pre-commit hook and the pre-commit.ci bot if you chose to implement that | ||
too. | ||
|
||
```yaml | ||
# file: .pre-commit-config.yaml | ||
|
||
repos: | ||
- repo: https://github.com/PyCQA/isort | ||
rev: 5.11.4 | ||
hooks: | ||
- id: isort | ||
files: \.py$ | ||
|
||
# Misc commit checks using built in pre-commit checks | ||
- repo: https://github.com/pre-commit/pre-commit-hooks | ||
rev: v4.4.0 | ||
# ref: https://github.com/pre-commit/pre-commit-hooks#hooks-available | ||
hooks: | ||
# Autoformat: Makes sure files end in a newline and only a newline. | ||
- id: end-of-file-fixer | ||
|
||
# Lint: Check for files with names that would conflict on a | ||
# case-insensitive filesystem like MacOS HFS+ or Windows FAT. | ||
- id: check-case-conflict | ||
- id: trailing-whitespace | ||
|
||
# Linting: Python code (see the file .flake8) | ||
- repo: https://github.com/PyCQA/flake8 | ||
rev: "6.0.0" | ||
hooks: | ||
- id: flake8 | ||
|
||
# Black for auto code formatting | ||
repos: | ||
- repo: https://github.com/psf/black | ||
rev: 22.12.0 | ||
hooks: | ||
- id: black | ||
language_version: python3.8 | ||
|
||
# Tell precommit.ci bot to update codoe format tools listed in the file | ||
# versions every quarter | ||
# The default it so update weekly which is too many new pr's for many | ||
# maintainers (remove these lines if you aren't using the bot!) | ||
ci: | ||
autoupdate_schedule: quarterly | ||
``` | ||
:::{literalinclude} ../examples/pure-hatch/.pre-commit-config.yaml | ||
:language: yaml | ||
::: | ||
|
||
This file specifies a hook that will be triggered automatically before each `git commit`, | ||
in this case, it specifies a `flake8` using version `6.0.0`. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ucodery i've been thinking a bit about what code formatters we want to recommend. I know we suggested black and flake8 but that was last year! it seems to me a lot of people are using ruff these days! we just moved to it at stravalib a few months ago.
do you think we should consider a future issue / task for the future of suggesting ruff in place of these since it does so many things and creates a simpler / faster config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also moved a few project to ruff this year. I think it is a great tool, and like hatch it enmpaases a lot of tasks that previously required a medley of tools and decisions by users, which can be great for beginners. I think we should however raise this in a new issue/ discussion.