Consistent formatting (Python Black)

[billbrod]

Given that we have different authors with different styles, our formatting/conventions are not consistent. We should look into using something like yapf or black to make our code consistent and maybe pydocstyle for our documentation.

[billbrod]

As part of this, also:

• - make sure names are properly formatted (snake_case for variables, etc)
• - add linter (for code and docstrings) to CI

[billbrod]

After everything else has stabilized, run this on everything.

[NickleDave]

Chiming in here from the pyOpenSci review.
We do require this--indirectly--by asking that linting checks pass.
As you comment above, I would address other issues first and then add linting.
There are many ways to skin this cat, see https://learn.scientific-python.org/development/guides/style/
I use a nox session so anyone can run linting checks locally without learning the pre-commit CLI but you could also just let contributors remain mostly blissfully ignorant of linting, and run pre-commit in CI on PRs, etc.
How you do it is up to you but there should be some linting checks.

[billbrod]

I'm also looking into ruff for this, which might be nicer.

As mentioned above, will need linters to be added to CI as well (see #190 )

[hmd101]

Summary of Considerations so far:

We are currently looking at Ruff, nox and pre-commit hook to introduce formatting conventions. Below is a list of considerations aimed at guiding the decision process, as well as a list of their respective configurations and a brief explanation of each.

Ruff Linting:

• Provides a structured approach to integrating and configuring Flake8 plugins (see below).
• Simplifies the setup and management of Flake8 configurations.
• Streamlines linting setup, provides a unified configuration approach.
• Ruff has been chosen for its speed and efficiency in identifying common Python code issues.
• For Ruff-linter commands and rule-selection, check here.

Flake8:

• General-purpose Python linting tool
• Flake8 itself doesn't do any actual linting; instead, it acts as a wrapper that integrates multiple Python linting libraries and plugins, such as PyFlakes, and pycodestyle.

Considered Configurations for Ruff:

(in pyproject.toml; Legend: ✅: common and included, ❓ common but not included)

TL;DR:

• E, F, W are standard flake8 checks, that have stood the test of time. ✅

More details:

• D: ✅
  Flake8 can check for compliance with docstring conventions, such as missing docstrings or formatting issues.
• E: ✅
  General errors in the code detected by Flake8. This can include syntax errors, indentation errors, or other fundamental issues that prevent Python code from running correctly.
• F: ✅
  Flake8 uses this to indicate violations of code formatting conventions. This might include issues like incorrect indentation, incorrect line breaks, or other stylistic issues that do not affect code functionality but are recommended for consistency.
• W: ✅ These are warnings that Flake8 raises. Warnings typically point out potential issues or suggest improvements in your code that do not necessarily prevent it from running but can lead to better practices.
• B (flake8-bugbear): ✅
  Checks for common programming errors, style issues, or patterns that are error-prone
• I (isort): ✅ 
  Helps in managing import statements and ensuring they are organized according to a specified convention.
• ARG (flake8-unused-arguments): ❓
  Flags unused function arguments.
• C4 (flake8-comprehensions): ❓
  Checks for issues in list comprehensions and generator expressions.
• EM (flake8-errmsg): ❓
  Lints error message formatting.
• ICN (flake8-import-conventions): ❓
  Ensures imports are formatted according to best practices.
• PTH (flake8-use-pathlib): ❓
  Encourages the use of pathlib instead of os.path.
• RET (flake8-return):❓
  Checks for issues related to return statements.
• SIM (flake8-simplify): ❓
  Suggests simplifications for code.
• T20 (flake8-print): ❓
  Flags print statements.
• UP (pyupgrade): ❓
  Upgrades syntax to modern Python standards.
• YTT (flake8-2020):❓
  Enforces Python 3.8+ compatibility.

Nox & Pre-commit Hooks:

TL;DR:

• Both tools are complementary and contribute to better maintainability by promoting consistent coding practices and making it easier to identify and fix issues early in the development process. Both are recommended by the Scientific Python Library Development Guide.
• Nox: A task-runner automating testing across different environments after commits, ensuring code works well in varied setups specified in noxfile.py.
• Pre-commit: Ensures code quality before commits by enforcing checks like formatting and linting.

Nox

Nox is a Python-based task runner that is used to automate various development workflows, such as testing, building, and deploying applications. It allows you to define sessions, which are Python functions in the noxfile.py decorated with @nox.session with various parameters, such as name, by which these sessions can then be executed via the command line. This easily allows to test code by running pre-defined tasks with a single command across multiple Python versions without manually setting up environments.

Example: Based on the following noxfile.py , the command nox -s tests would run the session named "tests” below. nox without any commands would run all the sessions defined in the noxfile.py .

@nox.session(name="tests", python=["3.10", "3.11", "3.12"])
def tests(session):
    # run tests
    session.install("pytest")
    session.run("pytest")

Pre-Commit

Hook:
A pre-commit hook can be added to run Ruff linting on code before it is committed. This ensures that developers catch and fix linting issues early in the development process. It only runs when installed and can be bypassed with git commit -m <my commit message> --no-verify.

Considered Configurations: (in .pre-commit.yaml:
(Legend: ✅: included, ❓: recommended, but not included)

• check-added-large-files ✅
  • Purpose: Checks for large files added to the repository, typically to prevent accidental inclusion of large binaries or datasets.
• check-case-conflict ✅
  • Purpose: Detects potential filename conflicts due to case-insensitive filesystems (e.g., Windows) where File.txt and file.txt would be considered the same
• check-merge-conflict ✅
  • Purpose: Checks for leftover merge conflict markers (<<<<<<<, =======, >>>>>>>) in files.
• check-symlinks ❓
  • Purpose: Verifies that symbolic links are valid and do not point to non-existent targets.
• check-yaml✅
  • Purpose: Validates YAML files for syntax errors.
• debug-statements ✅
  • Purpose: Detects debug statements (e.g., print, console.log) left in code.
• end-of-file-fixer✅
  • Purpose: Ensures files have a newline at the end.
• mixed-line-ending❓
  • Purpose: Checks for files with mixed line endings (e.g., both LF and CRLF).
    • Pros: Prevents compatibility issues across platforms, ensures consistent line endings.
    • Cons: Can require adjustments for projects that intentionally use mixed line endings for specific reasons.
• requirements-txt-fixer❓
  • Purpose: Standardizes the format of requirements.txt files, ensuring they follow conventions and are easy to manage.
  • Requires adjustments for projects with complex dependency requirements.
• trailing-whitespace✅
  • Purpose: Removes trailing whitespace characters from files.

CI Pipeline:

Finally, the CI configuration ci.yml has been updated to include a step each for running Ruff formatting and linting check on all new commits and pull requests. Two separate actions were chosen to signal the developer which combination of the two might be causing an error.

[hmd101]

Now, that the ruff linter and formatter have been introduced (see PR #266), possible next steps to consider are as follows:

Note that if additional linting rules are to be introduced (e.g., B), each rule should be introduced in a separate PR. Therefore, we may want to consider closing this big issue and using my notes below and putting them in separate smaller issues.

Next steps:

0. restructure contributing file (see issue #296)

1.Ruff Linting

Include the following linting rules (in pyproject.toml ) and resolve linting errors:

• B (flake8-bugbear):
  Checks for common programming errors, style issues, or patterns that are error-prone
• ARG (flake8-unused-arguments):
  Flags unused function arguments.
• C4 (flake8-comprehensions):
  Checks for issues in list comprehensions and generator expressions.
• EM (flake8-errmsg):
  Lints error message formatting.
• ICN (flake8-import-conventions):
  Ensures imports are formatted according to best practices.
• PTH (flake8-use-pathlib):
  Encourages the use of pathlib instead of os.path.
• RET (flake8-return):
  Checks for issues related to return statements.
• SIM (flake8-simplify):
  Suggests simplifications for code.
• T20 (flake8-print):
  Flags print statements.
• UP (pyupgrade):
  Upgrades syntax to modern Python standards.
• YTT (flake8-2020):
  Enforces Python 3.8+ compatibility.

2. Nox:

We might want to consider to include nox sessions for:

• format: Runs code formatting too
• docs: Builds and verifies the project's documentation to ensure it's up to date and properly rendered.
• type check: uses mypy or a similar tool to perform static type checking, to catch type-related errors early.

3. Pre-commit

We might want to add the following to .pre-commit-config.yaml :

• mixed-line-ending
  • Purpose: Checks for files with mixed line endings (e.g., both LF and CRLF).
    • Pros: Prevents compatibility issues across platforms, ensures consistent line endings.
    • Cons: Can require adjustments for projects that intentionally use mixed line endings for specific reasons.
• requirements-txt-fixer
  • Purpose: Standardizes the format of requirements.txt files, ensuring they follow conventions and are easy to manage.
  • Requires adjustments for projects with complex dependency requirements.