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

Suppress stderr in call to ruff and add ruff formatter #333

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

felix-cw
Copy link
Contributor

This PR adds the fix I suggested in #331 to ensure compatibility with ruff 0.1.0+. I've confirmed it works with version 0.1.15.

I also added the ruff formatter itself, as a substitute for black, called "ruffformat". The formatter simply called "ruff" is for applying automatic fixes for diagnostics. For example, isort-style import sorting.

@MordorianGuy
Copy link

Would it be more adaptive to initialise the command depending on the ruff version which may be acquired via subprocess.run?
By the way, could you, please, elucidate a little how to install the package during development? When I install the released version of package via pip into a hollow environment (mamba create -n test pip) it installs without any problem. But when I am trying to install it either from my fork repository (via git+[link] notation) or directly from a local path (pip install -e [path]) the installation fails.

@felix-cw
Copy link
Contributor Author

Hi. Thanks for the comment. I believe the added flags for the ruff --fix command will work for versions pre 0.1.0 so no change required. Since ruff formatting was added more recently, the ruff command would fail for previous versions that do not support it. Since using these formatters is opt-in, we might expect someone who turns it on to have a compatible ruff version installed. Is there a way to warn the user they need to update their ruff version to use the feature?

On the installation, honestly I just edited the files in my conda env and transcribed the changes here. I know that's not right!
The doc site has a page about installing it for development but I haven't tried it yet.

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.

None yet

2 participants