Check adherence with sp-repo-review guidelines

[agriyakhetarpal]

There are a lot of interesting things available in the Scientific Python ecosystem, perhaps we should also try using sp-repo-review?

Originally posted by @agriyakhetarpal in #3390 (comment)

[agriyakhetarpal]

Output from running sp-repo-review locally

General:

 • Detected build backend: MISSING                                                                                                      

├── PY001 Has a pyproject.toml ❌
│   All projects should have a pyproject.toml file to support a modern build system and support wheel installs properly.                
├── PY002 Has a README.(md|rst) file ✅
├── PY003 Has a LICENSE* file ✅
├── PY004 Has docs folder ✅
├── PY005 Has tests folder ✅
├── PY006 Has pre-commit config ✅
└── PY007 Supports an easy task runner (nox or tox) ✅

PyProject:
├── PP002 Has a proper build-system table [skipped]
├── PP003 Does not list wheel as a build-dep [skipped]
├── PP301 Has pytest in pyproject [skipped]
├── PP302 Sets a minimum pytest to at least 6 [skipped]
├── PP303 Sets the test paths [skipped]
├── PP304 Sets the log level in pytest [skipped]
├── PP305 Specifies xfail_strict [skipped]
├── PP306 Specifies strict config [skipped]
├── PP307 Specifies strict markers [skipped]
├── PP308 Specifies useful pytest summary [skipped]
└── PP309 Filter warnings specified [skipped]

GitHub Actions:
├── GH100 Has GitHub Actions config ✅
├── GH101 Has nice names ✅
├── GH102 Auto-cancel on repeated PRs ✅
├── GH103 At least one workflow with manual dispatch trigger ✅
├── GH200 Maintained by Dependabot ✅
├── GH210 Maintains the GitHub action versions with Dependabot ✅
└── GH211 Do not pin core actions as major versions ✅

Pre-commit:
├── PC100 Has pre-commit-hooks ✅
├── PC110 Uses black or ruff-format ❌
│   Must have https://github.com/psf/black-pre-commit-mirror or https://github.com/astral-sh/ruff-pre-commit + id: ruff-format in       
│   .pre-commit-config.yaml                                                                                                             
├── PC111 Uses blacken-docs [skipped]
├── PC140 Uses mypy ❌
│   Must have https://github.com/pre-commit/mirrors-mypy repo in .pre-commit-config.yaml                                                
├── PC160 Uses codespell ❌
│   Must have https://github.com/codespell-project/codespell repo in .pre-commit-config.yaml                                            
├── PC170 Uses PyGrep hooks (only needed if RST present) ✅
├── PC180 Uses prettier ❌
│   Must have https://github.com/pre-commit/mirrors-prettier repo in .pre-commit-config.yaml                                            
├── PC190 Uses Ruff ✅
├── PC191 Ruff show fixes if fixes enabled ❌
│   If --fix is present, --show-fixes must be too.                                                                                      
└── PC901 Custom pre-commit CI message ✅

MyPy:
├── MY100 Uses MyPy (pyproject config) [skipped]
├── MY101 MyPy strict mode [skipped]
├── MY102 MyPy show error codes [skipped]
├── MY103 MyPy warn unreachable [skipped]
├── MY104 MyPy enables ignore-without-code [skipped]
├── MY105 MyPy enables redundant-expr [skipped]
└── MY106 MyPy enables truthy-bool [skipped]

Ruff:
├── RF001 Has Ruff config [skipped]
├── RF002 Target version must be set [skipped]
├── RF003 src directory specified if used [skipped]
├── RF101 Bugbear must be selected [skipped]
├── RF102 isort must be selected [skipped]
├── RF103 pyupgrade must be selected [skipped]
├── RF201 Avoid using deprecated config settings [skipped]
└── RF202 Use (new) lint config section [skipped]

Documentation:
├── RTD100 Uses ReadTheDocs (pyproject config) ✅
├── RTD101 You have to set the RTD version number to 2 ✅
├── RTD102 You have to set the RTD build image ✅
└── RTD103 You have to set the RTD python version ✅

The build-system check will be fixed when #3301 is merged and related checks that are skipped can be checked again. @Saransh-cpp reports that we can skip using isort, codespell, and related tools in #3390 (comment). Type checkers like MyPy can wait and can be revisited once type hints are added for PyBaMM models. For ruff-related checks, we have #3477 which a PR for can tackle.

[agriyakhetarpal]

Here's an updated output from sp-repo-review:

Expand to view

General:

 • Detected build backend: setuptools.build_meta                                                                                                                  
 • Detected license(s): BSD License                                                                                                                               

├── PY001 Has a pyproject.toml ✅
├── PY002 Has a README.(md|rst) file ✅
├── PY003 Has a LICENSE* file ✅
├── PY004 Has docs folder ✅
├── PY005 Has tests folder ✅
├── PY006 Has pre-commit config ✅
└── PY007 Supports an easy task runner (nox or tox) ✅

PyProject:
├── PP002 Has a proper build-system table ✅
├── PP301 Has pytest in pyproject ✅
├── PP302 Sets a minimum pytest to at least 6 ✅
├── PP303 Sets the test paths ✅
├── PP304 Sets the log level in pytest ✅
├── PP305 Specifies xfail_strict ✅
├── PP306 Specifies strict config ✅
├── PP307 Specifies strict markers ✅
├── PP308 Specifies useful pytest summary ✅
└── PP309 Filter warnings specified ✅

GitHub Actions:
├── GH100 Has GitHub Actions config ✅
├── GH101 Has nice names ✅
├── GH102 Auto-cancel on repeated PRs ✅
├── GH103 At least one workflow with manual dispatch trigger ✅
├── GH104 Use unique names for upload-artifact ❌
│   Multiple upload-artifact usages must have unique names to be compatible with v4 (which no longer merges artifacts, but instead errors out). The most general  
│   solution is:                                                                                                                                                  
│   
│                                                                                                                                                                 
│    - uses: actions/upload-artifact@v4                                                                                                                           
│      with:                                                                                                                                                      
│        name: prefix-${{ github.job }}-${{ strategy.job-index }}                                                                                                 
│                                                                                                                                                                 
│    - uses: actions/download-artifact@v4                                                                                                                         
│      with:                                                                                                                                                      
│        pattern: prefix-*                                                                                                                                        
│        merge-multiple: true                                                                                                                                     
│                                                                                                                                                                 
│   
│    • No variable substitutions were detected in publish_pypi.yml:build_macos_and_linux_wheels.                                                                  
├── GH200 Maintained by Dependabot ✅
├── GH210 Maintains the GitHub action versions with Dependabot ✅
├── GH211 Do not pin core actions as major versions ✅
└── GH212 Require GHA update grouping ✅

Pre-commit:
├── PC100 Has pre-commit-hooks ✅
├── PC110 Uses black or ruff-format ✅
├── PC111 Uses blacken-docs ✅
├── PC140 Uses mypy ❌
│   Must have https://github.com/pre-commit/mirrors-mypy repo in .pre-commit-config.yaml                                                                          
├── PC160 Uses codespell ❌
│   Must have https://github.com/codespell-project/codespell repo in .pre-commit-config.yaml                                                                      
├── PC170 Uses PyGrep hooks (only needed if RST present) ✅
├── PC180 Uses prettier ❌
│   Must have https://github.com/pre-commit/mirrors-prettier repo in .pre-commit-config.yaml                                                                      
├── PC190 Uses Ruff ✅
├── PC191 Ruff show fixes if fixes enabled ✅
└── PC901 Custom pre-commit CI message ✅

MyPy:
├── MY100 Uses MyPy (pyproject config) ✅
├── MY101 MyPy strict mode ❌
│   Must have strict in the mypy config. MyPy is best with strict or nearly strict configuration. If you are happy with the strictness of your settings already,  
│   ignore this check or set strict = false explicitly.                                                                                                           
│   
│                                                                                                                                                                 
│    [tool.mypy]                                                                                                                                                  
│    strict = true                                                                                                                                                
│                                                                                                                                                                 
├── MY102 MyPy show_error_codes deprecated ✅
├── MY103 MyPy warn unreachable ❌
│   Must have warn_unreachable (true/false) to pass this check. There are occasionally false positives (often due to platform or Python version static checks), so
│   it's okay to set it to false if you need to. But try it first - it can catch real bugs too.                                                                   
│   
│                                                                                                                                                                 
│    [tool.mypy]                                                                                                                                                  
│    warn_unreachable = true                                                                                                                                      
│                                                                                                                                                                 
├── MY104 MyPy enables ignore-without-code ❌
│   Must have "ignore-without-code" in enable_error_code = [...]. This will force all skips in your project to include the error code, which makes them more      
│   readable, and avoids skipping something unintended.                                                                                                           
│   
│                                                                                                                                                                 
│    [tool.mypy]                                                                                                                                                  
│    enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"]                                                                                 
│                                                                                                                                                                 
├── MY105 MyPy enables redundant-expr ❌
│   Must have "redundant-expr" in enable_error_code = [...]. This helps catch useless lines of code, like checking the same condition twice.                      
│   
│                                                                                                                                                                 
│    [tool.mypy]                                                                                                                                                  
│    enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"]                                                                                 
│                                                                                                                                                                 
└── MY106 MyPy enables truthy-bool ❌
    Must have "truthy-bool" in enable_error_code = []. This catches mistakes in using a value as truthy if it cannot be falsey.                                   
    
                                                                                                                                                                  
     [tool.mypy]                                                                                                                                                  
     enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"]                                                                                 
                                                                                                                                                                  

Ruff:
├── RF001 Has Ruff config ✅
├── RF002 Target version must be set ✅
├── RF003 src directory specified if used [skipped]
├── RF101 Bugbear must be selected ✅
├── RF102 isort must be selected ❌
│   Must select the isort I checks. Recommended:                                                                                                                  
│   
│                                                                                                                                                                 
│    [tool.ruff.lint]                                                                                                                                             
│    extend-select = [                                                                                                                                            
│      "I",  # isort                                                                                                                                              
│    ]                                                                                                                                                            
│                                                                                                                                                                 
├── RF103 pyupgrade must be selected ✅
├── RF201 Avoid using deprecated config settings ✅
└── RF202 Use (new) lint config section ✅

Documentation:
├── RTD100 Uses ReadTheDocs (pyproject config) ✅
├── RTD101 You have to set the RTD version number to 2 ✅
├── RTD102 You have to set the RTD build image ✅
└── RTD103 You have to set the RTD python version ✅

@Saransh-cpp, it looks like we have a few MyPy-related issues here, could you ensure that they are fixed (or addressed as best they can based on feasibility) when you return to #3853?

P.S. I'll take up the "Have unique names for artifacts" point soon.

[Rishab87]

Hi, I see this is pending from a long time I would like to take upon the mypy part, can this issue be assigned to me?

[Saransh-cpp]

We will have a list of things to fix once #4897 is merged -

[tool.repo-review]
ignore = [
    "MY",    # ignore all MyPy setting checks (MY101, MY103, MY104, MY105, MY106)
    "PP004", # ignore the check that prevents an upper cap on Python requires
    "PP308", # ignore the explicit pytest summary flag (-ra) check
    "PC140", # ignore missing type checker in pre-commit
    "PC160", # ignore missing spell checker in pre-commit
    "PC180", # ignore missing markdown formatter in pre-commit
    "RF102", # ignore the missing isort selection in Ruff configuration
]