Opened 2 years ago

Closed 2 years ago

#30503 closed enhancement (fixed)

src/tox.ini: Add environment codespell

Reported by: Samuel Lelièvre Owned by:
Priority: major Milestone: sage-9.2
Component: documentation Keywords: documentation, spelling, spellcheck
Cc: Frédéric Chapoton, Samuel Lelièvre, Tobias Diez Merged in:
Authors: Matthias Koeppe Reviewers: Tobias Diez
Report Upstream: N/A Work issues:
Branch: e5a916f (Commits, GitHub, GitLab) Commit: e5a916fbedee067b8a58b0d640f7465d2269668c
Dependencies: #30467 Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

We add a tox environment codespell.

To try: ./sage -tox -e codespell

Change History (22)

comment:1 Changed 2 years ago by Matthias Köppe

Dependencies: #30467
Description: modified (diff)

comment:2 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/apply_fossies_codespell

comment:3 Changed 2 years ago by Matthias Köppe

Commit: 63099101c2eb3ddbefde322f6f033116917ecbdd
Description: modified (diff)

New commits:

df7ef3fMerge branch 't/30410/command__sage__tox_' into t/30467/src_tox_ini__check_patchbot_plugin_patterns
7e9d12dsrc/tox.ini: New environment relint
4a3a33aAdd comment
b0ad03esrc/bin/sage: Show tox environment list in 'sage -advanced'
7cf9efesrc/.relint.yml: Add pattern from #30472
62e342cMerge branch 't/30467/src_tox_ini__check_patchbot_plugin_patterns' into t/30503/apply_fossies_codespell
6309910src/tox.ini: Add codespell

comment:4 Changed 2 years ago by git

Commit: 63099101c2eb3ddbefde322f6f033116917ecbddca64a5276938d2bfb41823111289fa65bee182b3

Branch pushed to git repo; I updated commit sha1. New commits:

ca64a52src/tox.ini (codespell): Skip non-English doc, binary files, backup files

comment:5 Changed 2 years ago by git

Commit: ca64a5276938d2bfb41823111289fa65bee182b3d8e101cff60472aa971b075e96895eeebe4adf12

Branch pushed to git repo; I updated commit sha1. New commits:

d8e101csrc/.codespell-dictionary.txt: New, consider these words correct

comment:6 Changed 2 years ago by Matthias Köppe

Authors: Matthias Koeppe
Status: newneeds_review
Summary: Apply fossies codespellsrc/tox.ini: Add environment codespell
Last edited 2 years ago by Matthias Köppe (previous) (diff)

comment:7 Changed 2 years ago by Matthias Köppe

Cc: Tobias Diez added

comment:8 Changed 2 years ago by git

Commit: d8e101cff60472aa971b075e96895eeebe4adf12e3273142286eabd9d2537a7d22866bf40ae1bfba

Branch pushed to git repo; I updated commit sha1. New commits:

e327314src/bin/sage (-advanced): Generalize tox environment list formatting

comment:9 Changed 2 years ago by Tobias Diez

I like it! A few observations:

  • Documentation needed, especially how it integrates with common IDEs.
  • Is the idea to run it as part of a github action, or patchbot on each ticket?
  • The exceptions added in .codespell-dictionary.txt seem to be proper missspellings or abbreviations that in my opinion shouldn't be encouraged in a multi-language dev team. I would actually propose to add the conversions ans -> answer, numer -> number as a custom codespell directory (- D argument)
  • What's the reason to only apply it to the source folder, and not the whole project (including build scripts etc)
  • Rename codespell-dictinary.txt to codespell-ignore. In codespell terminology, a dictionary flags misspellings and provides alternatives, e.g https://github.com/codespell-project/codespell/blob/45b67b427489910b0ef5e4566c9b3989be3f0f33/codespell_lib/data/dictionary_rare.txt
  • Is it possible to add the configuration as a config block in tox instead of as cmd line args? https://github.com/codespell-project/codespell#using-a-config-file wasn't clear about tox support for this.
Last edited 2 years ago by Tobias Diez (previous) (diff)

comment:10 Changed 2 years ago by Matthias Köppe

Status: needs_reviewneeds_work

Thanks a lot for the comments!

comment:11 in reply to:  9 Changed 2 years ago by Matthias Köppe

Replying to gh-tobiasdiez:

It only checks for .codespellrc and setup.cfg. I'm going to use .codespellrc

comment:12 Changed 2 years ago by Matthias Köppe

Actually, the released version (1.17.1) has no support for config files at all, so we have to keep the command line.

comment:13 Changed 2 years ago by git

Commit: e3273142286eabd9d2537a7d22866bf40ae1bfba2ff29857b4537ecb4b4f57cbfa981af5fd372f3f

Branch pushed to git repo; I updated commit sha1. New commits:

0847d49tox.ini: When delegating to src/tox.ini, use -- as a separator
2ff2985src/.codespell-ignore.txt: New; use in addition to .codespell-dictionary.txt

comment:14 Changed 2 years ago by git

Commit: 2ff29857b4537ecb4b4f57cbfa981af5fd372f3fcaba2876c789e05889e86c6d7b67f89c4c8458b6

Branch pushed to git repo; I updated commit sha1. New commits:

92c3b3dtox.ini (codespell): If invoked from SAGE_ROOT, codespell the whole Sage distribution by default
caba287src/tox.ini (codespell): Skip more files and directories

comment:15 in reply to:  9 Changed 2 years ago by Matthias Köppe

Replying to gh-tobiasdiez:

  • What's the reason to only apply it to the source folder, and not the whole project (including build scripts etc)

src/ is the "pure Python" part of the project (sagelib). Most Sage developers are only familiar with this part.

But I have changed the top-level tox.ini so that it runs codespell on the same distribution.

Many of the excluded file/directory patterns could actually be obtained from .gitignore - but then again we would need a shell script to do this. I think it's "good enough" for this ticket.

comment:16 in reply to:  9 Changed 2 years ago by Matthias Köppe

Status: needs_workneeds_review

Replying to gh-tobiasdiez:

  • Documentation needed, especially how it integrates with common IDEs.

Let's add documentation in #30453 (sage -advanced already shows it as part of the help). I will need help there with the IDE angle - I only use the command line.

  • Is the idea to run it as part of a github action, or patchbot on each ticket?

As of this ticket, it's just a dev tool that can be run locally. I don't have plans to work on CI integration for it.

comment:17 Changed 2 years ago by git

Commit: caba2876c789e05889e86c6d7b67f89c4c8458b6e5a916fbedee067b8a58b0d640f7465d2269668c

Branch pushed to git repo; I updated commit sha1. New commits:

e5a916ftox.ini: Add codespell to envlist

comment:18 Changed 2 years ago by Tobias Diez

Thanks! LGTM

(By the way, what are the conventions concerning putting yourself as reviewer, and changing the status to positive review?)

Last edited 2 years ago by Tobias Diez (previous) (diff)

comment:19 Changed 2 years ago by Matthias Köppe

As soon as you have participated in reviewing a ticket, it is appropriate to add your name to the list of reviewers.

(I have started to use the format "My name, ..." to indicate that more reviewers may be needed to cover other aspects of the ticket that I do not feel confident (or do not have time) to review. But this is not widely used ... yet.)

If you are the only reviewer and you are confident about giving a positive review, you set positive_review. If there are several active reviewers, some communication using comments is needed.

comment:20 Changed 2 years ago by Matthias Köppe

Reviewers: Tobias Diez
Status: needs_reviewpositive_review

Thanks for the review.

comment:21 Changed 2 years ago by Tobias Diez

Thanks to you, for the explanation!

comment:22 Changed 2 years ago by Volker Braun

Branch: u/mkoeppe/apply_fossies_codespelle5a916fbedee067b8a58b0d640f7465d2269668c
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.