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

Use Ruff formatting and linting. Install configuration moved to pyproject.toml #1323

Merged
merged 25 commits into from
Feb 12, 2025

Conversation

keileg
Copy link
Contributor

@keileg keileg commented Feb 6, 2025

Proposed changes

This PR suggests two changes to how the PorePy project is installed and managed:

  1. Information for setup/install and configuration for static checks (mypy et al) is moved from the files setup.py, setup.cfg and requirements.txt to pyproject.toml. While this is not an urgent update, it has been the recommended way of managing python projects for some years now.
  2. Formatting and linting, previously handled by flake8 and black, respectively, is moved to ruff. This is primarily for the reason of speed. Note that all changes in Python files (except from setup.py) in this PR are due to this change, with formatting being the main culprit. UPDATE: Mypy forced a few additional updates, but ruff formatting is still by far the main contributor.
    Both these updates necessitated updates to the GH action files as well.

@IvarStefansson @jwboth Can you please have a look at the changes and let me know what you think? But do so after the tests have passed; I cannot I did not introduce bugs in GH action setups.

Types of changes

What types of changes does this PR introduce to PorePy?
Put an x in the boxes that apply.

  • Minor change (e.g., dependency bumps, broken links).
  • Bugfix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Testing (contribution related to testing of existing or new functionality).
  • Documentation (contribution related to adding, improving, or fixing documentation).
  • Maintenance (e.g., improve logic and performance, remove obsolete code).
  • Other: Setup, formatting, GH actions

Checklist

Put an x in the boxes that apply or explain briefly why the box is not relevant.

  • The documentation is up-to-date.
  • Static typing is included in the update.
  • This PR does not duplicate existing functionality.
  • The update is covered by the test suite (including tests added in the PR).
  • If new skipped tests have been introduced in this PR, pytest was run with the --run-skipped flag.

@keileg
Copy link
Contributor Author

keileg commented Feb 6, 2025

@jwboth @IvarStefansson: Tests are finally passing, this should be ready for review now.

Copy link
Contributor

@IvarStefansson IvarStefansson left a comment

Choose a reason for hiding this comment

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

I like it. Am I right ruff is somewhat stricter on blank lines than black?

.github/workflows/run-pytest-all.yml Show resolved Hide resolved
.github/workflows/run-static-checks.yml Outdated Show resolved Hide resolved
src/porepy/fracs/fracture_network_2d.py Show resolved Hide resolved
src/porepy/fracs/fracture_network_3d.py Show resolved Hide resolved
src/porepy/fracs/meshing.py Outdated Show resolved Hide resolved
src/porepy/fracs/split_grid.py Outdated Show resolved Hide resolved
src/porepy/fracs/structured.py Outdated Show resolved Hide resolved
src/porepy/models/fluid_property_library.py Outdated Show resolved Hide resolved
src/porepy/numerics/fv/tpsa.py Outdated Show resolved Hide resolved
@keileg
Copy link
Contributor Author

keileg commented Feb 6, 2025

I like it. Am I right ruff is somewhat stricter on blank lines than black?

That could be, I have honestly not looked at the detailed reformatting. Life got easier when I outsourced all thinking about formatting to black, and that is a habit that is hard to break.

Copy link
Contributor

@jwboth jwboth left a comment

Choose a reason for hiding this comment

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

Went through the changes, and have no objections. Ruff looks good to me.

@IvarStefansson IvarStefansson self-requested a review February 11, 2025 10:35
@keileg keileg merged commit 0f77de0 into develop Feb 12, 2025
6 checks passed
@keileg keileg deleted the ruff branch February 12, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants