Skip to content

Conversation

@seanbudd
Copy link
Contributor

@seanbudd seanbudd commented May 20, 2025

We encourage merging this upstream to prevent merge conflicts with our fork.

Please also consider setting up pre-commit CI for this repo to ensure any changes on this fork conform to our linting standards.
I would also suggest setting up pyright, ruff, and pre-commit in your local dev environment

This PR includes the following changes from nvaccess#2:

  • The files .pre-commit-config.yaml and pyproject.toml were added. The structure of these files is kept close to that found in the corresponding files from the main NVDA repository, with some configurations removed that are not applicable to this repository.
  • The JSON configuration file for pyright was removed, since pyright configuration is now being handled by pyproject.toml.
  • The option reportMissingImports was temporarily set to false, since it was triggering pyright errors.
  • Some unnecessary noqa comments were removed, and one was added to suppress a pyright error caused by the scriptHandler.script decorator.
  • Whitespace was removed from the ends of lines, CRLF was converted to LF, and other minor formatting changes were carried out by running ruff-format.

Further changes from nvaccess#3:

  • Removed the lint action, since we are now using pre-commit.
  • Updated the zip Cargo dependency to 3.0.0 as the 2.x releases have been yanked
  • Added rust files to pre-commit whitespace fixing

RyanMcCleary and others added 2 commits May 20, 2025 16:24
This PR includes the following changes:

The files .pre-commit-config.yaml and pyproject.toml were added. The structure of these files is kept close to that found in the corresponding files from the main NVDA repository, with some configurations removed that are not applicable to this repository.
The JSON configuration file for pyright was removed, since pyright configuration is now being handled by pyproject.toml.
The option reportMissingImports was temporarily set to false, since it was triggering pyright errors.
Some unnecessary noqa comments were removed, and one was added to suppress a pyright error caused by the scriptHandler.script decorator.
Whitespace was removed from the ends of lines, CRLF was converted to LF, and other minor formatting changes were carried out by running ruff-format.
Note: To run the pre-commit checks, install pyright, ruff, and pre-commit in a virtual environment first.
@NSoiffer
Copy link
Collaborator

I use vscode and there is a .vscode/setting.json file. It has some flake8 settings which probably need to be deleted. In my environment, I have ruff and pylance (which uses pyright) installed. What probably isn't correct is the settings for these. I see you added something for pyright, but does something need to be added for ruff?

Can you add/change the appropriate settings in .vscode/setting.json along with whatever should be in the requirements.txt file (I think) for pre-commit. I haven't used that before but it does sound useful.

@seanbudd
Copy link
Contributor Author

I'll ask @RyanMcCleary to update .vscode/setting.json.
This repo doesn't appear to use a requirements.txt - instead we've noted dependencies in pyproject.toml

@NSoiffer
Copy link
Collaborator

This repo doesn't appear to use a requirements.txt - instead we've noted dependencies in pyproject.toml

Does this mean I don't need to do anything for pre-commit or that I should create a requirements.txt file move the dependencies there along with adding something for pre-commit? Until just now, I wasn't aware of this file but it is certainly something that is useful.

For pre-commit, it looks there needs to be a .pre-commit-config.yaml file. Could you add something that is compatible with what NVDA uses?

@seanbudd
Copy link
Contributor Author

@NSoiffer - you need to install and set up pre-commit locally.
I don't know what system you prefer to deal with managing a python dev environment. We use uv - I can ask Ryan to setup uv for MathCAT if you wish.
I don't think a requirements.txt file should be used - but if that's your preference I can ask Ryan to move dependencies from pyproject.toml over to a requirements.txt file.

NVDA uses .pre-commit-config.yaml. Ryan has created rules in this .pre-commit-config.yaml which matches closely what we use in NVDA to make it so the final PR to NVDA passes our pre-commit checks.

RyanMcCleary and others added 2 commits May 21, 2025 12:19
This commit contains the following changes

Removed the lint action, since we are now using pre-commit.
Added trailing newlines to some files which did not previously have them.
Updated the zip Cargo dependency to 3.0.0 as the 2.x releases have been yanked
Added rust files to pre-commit whitespace fixing
@seanbudd seanbudd changed the title Updating pyproject.toml and .pre-commit-config.yaml (#2) Creating pyproject.toml and .pre-commit-config.yaml May 21, 2025
@NSoiffer NSoiffer merged commit 75e6853 into TalkingCatSW:main May 21, 2025
@NSoiffer
Copy link
Collaborator

Thanks for all your help. I think I told Sean a while back that python is far from my "first" language and so my build environment was rather ad-hoc.

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