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.

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

chore: remove references to black from the project #2674

Merged
merged 6 commits into from
Jul 21, 2024

Conversation

fpgmaas
Copy link
Contributor

@fpgmaas fpgmaas commented Jul 16, 2024

Description

There are a few mentions of black in the project, but they seem inconsistent;

  • In .github/workflows/docs.yml, we use black to validate the formatting of Python code in the docs. However, there is no set-up or instructions to format code locally. Of course this is not a big issue; fixing the formatting is trivial when this gives an error by installing black.
    - uses: psf/black@stable
      with:
        src: docs/src/python
  • In .github/workflows/python_build.yml black seems to be installed but unused.
  • There is some configuration for black in python/pyproject.toml, even though the formatter for the project is ruff.

This PR aims to solve these inconsistencies:

  • Remove the references to black listed above.
  • Use ruff as a formatter for the docs to be consistent with the python directory.
  • Add a small Makefile to the docs directory, with a format and a check command.

Considered alternative

I also considered adding the make commands to python/Makefile, similar to the build-docs command:

.PHONY: format-docs-py
format-docs-py: ## Format python files in the docs directory
    $(info --- format Python code in docs ---)
    (cd ../docs; ruff format .)

.PHONY: check-docs-py
check-docs-py: ## Check if python files in the docs directory are formatted
    $(info --- check Python code in docs ---)
    (cd ../docs; ruff format --check .)

However, this solution has the disadvantage that we need to either install the complete virtual environment in the docs.yml workflow, or hardcode the version of ruff there, e.g.

  lint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - run: |
          cd python
          # Should be the same as in python/pyproject.toml
          pip install ruff==0.5.2 
          make check-docs-py

@github-actions github-actions bot added the binding/python Issues for the Python package label Jul 16, 2024
@fpgmaas fpgmaas marked this pull request as draft July 16, 2024 17:16
rtyler
rtyler previously approved these changes Jul 16, 2024
@fpgmaas
Copy link
Contributor Author

fpgmaas commented Jul 16, 2024

I believe CI/CD is failing due to this issue. Merging this PR will solve it for now.

rtyler pushed a commit that referenced this pull request Jul 18, 2024
…ipeline (#2679)

Currently, `ruff` and `mypy` have their latest versions installed in the
CI pipeline, while locally they are fixed to a specific version. This
can cause issues, see #2678.

This PR proposes to fix them to their specific version in the pipeline.
The alternative I could think of was installing the virtual environment
with `make develop`, but that takes between 4 and 5 minutes, which might
be considered a bit too long to wait on linting results.

This PR will have conflicts with
#2674, so I'll need to rebase
one of these PR's once the other is merged.

# Related Issue(s)

- closes [#106](#2678)
@rtyler rtyler enabled auto-merge (rebase) July 18, 2024 10:29
auto-merge was automatically disabled July 18, 2024 10:42

Rebase failed

@ion-elgreco ion-elgreco enabled auto-merge (squash) July 21, 2024 20:22
@ion-elgreco ion-elgreco merged commit be6e372 into delta-io:main Jul 21, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/python Issues for the Python package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants