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

Setup Vale (prose linter) with a minimal unobtrusive configuration #281

Merged

Conversation

flpm
Copy link
Member

@flpm flpm commented May 23, 2024

What was done

  • Added vale as a dev dependency in pyproject.toml
  • Remove vale config file from git-ignore
  • Created the folder vale-styles to store vale style rules
  • Created a custom style rule set (package-guide-test) with one rule to check for incorrect capitalizations of PyPI (PyPI.yml)
  • Added the write-good rule set to serve as example (not enabled)
  • Created a very basic configuration file (.vale.ini)
  • Configured pre-commit to run vale
  • Used vale to fix all improper capitalizations found in the current MarkDown files
  • Excluded vale-styles folder from pre-commit codespell
  • Excluded vale-styles folder from sphinx build

The initial configuration is set to be unobtrusive: only the package-guide-test rule set is enabled and the PyPi rule generates warnings instead of errors (do not prevent commits).

With this configuration, vale can be run in the command line and in pre-commit, but pre-commit will not block commits that violate the rule because it is a warning.

Once the editorial team decides what style rules they would like to enforce, community-created rules can be installed or custom rules can be created in the vale-styles directory. I am happy to help with that if needed 🤚

An example of running vale in the command line

If you save the following content as test.md:

# Test for Vale

PYPI is not how you should spell PyPI. This will trigger a Vale error on line 3, character 1.

The rule also checks for the correct spelling of TestPYPI, which is TestPyPI. This line will
also trigger a Vale error on line 5, character 50.

The rule should not trigger in cases where a lowercase capitalization is correct,
like here: https://pypi.org or test-pypi-search

Then run vale passing the proper path, you will see 2 warnings:

$ vale test.md

 test.md
 3:1   warning  Consider using 'PyPI' instead   package-guide-test.PyPI 
                of 'PYPI'                                               
 5:50  warning  Consider using 'TestPyPI'       package-guide-test.PyPI 
                instead of 'TestPYPI'                                   

✖ 0 errors, 2 warnings and 0 suggestions in 1 file.

Editor integration

Another interesting option is to run vale in the actual editor. Vale provides integrations to a bunch of common editors (Obsidian, VS Code, Emacs, Vim, etc). Just like with code linters, you get immediate feedback while writing and fix issues before you commit your contribution.

Here is what it looks like on VS Code.

screenshot

For this, each author would have to install Vale, the appropriate editor extension and configure it for their workspace.

More information about vale

Homepage: https://vale.sh/
Documentation: https://vale.sh/docs/
Config details: https://vale.sh/docs/vale-cli/structure/#config
Community-created rule sets: https://vale.sh/explorer/

Fixes #236

@flpm flpm force-pushed the issue-236-config-and-enable-vale-in-pre-commit branch from f7dac7c to 7edd507 Compare May 23, 2024 17:49
@flpm
Copy link
Member Author

flpm commented May 23, 2024

pre-commit.ci autofix

@flpm flpm force-pushed the issue-236-config-and-enable-vale-in-pre-commit branch from 25ff5c1 to 3d01f59 Compare May 23, 2024 18:11
@flpm
Copy link
Member Author

flpm commented May 23, 2024

pre-commit.ci autofix

Copy link
Member

@lwasser lwasser left a comment

Choose a reason for hiding this comment

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

Thank you again !!! i think we can merge - but i'd love some help understanding how we expand our style guide. we will then use this in all of our repos! we had a lot of debates around how to spell out words like backend / back-end -- in the packaging build front and back end sections for instance!

@@ -104,7 +104,7 @@ exist in the default Anaconda cloud channel.

:::{figure-md} pypi-conda-channels

<img src="../images/python-pypi-conda-channels.png" alt="Graphic with the title Python package repositories. Below it says Anything hosted on PyPI can be installed using pip install. Packaging hosted on a conda channel can be installed using conda install. Below that there are two rows. the top row says conda channels. next to it are three boxes one with conda-forge, community maintained; bioconda and then default - managed by the anaconda team. Below that there is a row that says PyPI servers. PyPI - anyone can publish to pypi. and test pypi. a testbed server for you to practice. " width="700px">
<img src="../images/python-pypi-conda-channels.png" alt="Graphic with the title Python package repositories. Below it says Anything hosted on PyPI can be installed using pip install. Packaging hosted on a conda channel can be installed using conda install. Below that there are two rows. the top row says conda channels. next to it are three boxes one with conda-forge, community maintained; bioconda and then default - managed by the anaconda team. Below that there is a row that says PyPI servers. PyPI - anyone can publish to PyPI. and test PyPI. a testbed server for you to practice. " width="700px">
Copy link
Member

Choose a reason for hiding this comment

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

This is so so great!! i'll try to review and get this merged tomorrow @flpm thank you again. we have so many open issues and pr's it's taking a bit longer than expected to merge things!

@kierisi i just wanted you to see this - notice that Filipe setup a rule for pypi to be PyPI and it catches that issue across our guidebook! we can now create a style guide that is checked in CI using this tool (and we can do the same on our website too)!!

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the current PyPI rule is set to warning and not an error, pre-commit will succeed in CI. But if the level: warning is replaced with level: error in PyPI.yml, pre-commit will fail .

Copy link
Member

Choose a reason for hiding this comment

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

ahhhh ok perfect!

Copy link
Member

Choose a reason for hiding this comment

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

@flpm so is this file where we add words that might be unusual but we want to accept as a part of our vocabulary?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is right, this is a way to customize the internal dictionary. But the dictionary checks are part of the internal Vale ruleset that is currently disabled in .vale.ini because it triggers lots of errors. A better way to approach this is to use individual rules like the PyPI one.

@@ -0,0 +1,22 @@
extends: substitution
Copy link
Member

Choose a reason for hiding this comment

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

Can you also please help me understand how this file works? Is there a way we could just create a list of words that we'd always want to spell a certain way rather than having a yml file for each word (if that makes sense?). i may not understand.

so a list like this

pyopensci: pyOpenSci
PyOpenSci: pyOpenSci
pypi: PyPI
PYPI: PyPI
back end: back-end
backend: back-end

etc?
thank you again for this! i'm just trying to understand how this works!

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. There are multiple extension points that can be used to customize Vale.

This rule uses extends: substitution, which needs a section called swap that defines the mapping between incorrect strings and their correct versions.

The first part of the map can also be a regular expression. I had to use two regular expressions in the PyPI rule because otherwise the lowercase string pypi will match inside URLs https://pypi.org (false positive).

You are right about the list, to check for pyOpenSci case issues, you could either create a new rule like this one or even rename the file to something more generic like capitalizations.yml and extend the list here.

Likely you will run into similar false positives with the lowercase version of pyopensci matching URLs so you will need a regular expression to exclude them, but you could do something like I did for PyPI:

(?:\spyopenscy[\.,]?\s): pyOpenSci

(? ) tells Vale it is a regexp
\s matches a word boundary (space, tab)
[.,]? allow the existence of a period or comma

This regexp matches pyopensci and pyopensci. but not pyopensci.org

For other extension points the structure is slightly different, for example for extends: existence instead of swap you have tokens and that contains the list of words you want to warn users to avoid.

extends: existence
message: Consider removing '%s'
level: warning
ignorecase: true
tokens:
    - appears to be
    - arguably

Here are more details about the extension points https://vale.sh/docs/topics/styles/#extension-points

I put some example rules as examples (write-good ruleset) in ./vale-styles/write-good/ to illustrate what rules can do to and how the files are structured.

Copy link
Member

Choose a reason for hiding this comment

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

thank you so much!! this makes sense. and i hadn't considered the word within a url so regex also makes a lot of sense to control for that!

@kierisi
Copy link
Contributor

kierisi commented May 31, 2024

@all-contributors add @flpm for code

Copy link
Contributor

@kierisi

I've updated the pull request to add @flpm! 🎉

Copy link
Member

@lwasser lwasser left a comment

Choose a reason for hiding this comment

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

Ok let's see how this goes!! merging!!

@lwasser lwasser merged commit bc4776e into pyOpenSci:main Jun 4, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Use Vale (or some other tool) to check for style in a github action?
3 participants