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

maint: Add additional linter-related pre-commit hooks #3337

Merged
merged 6 commits into from
Oct 8, 2021

Conversation

Skylion007
Copy link
Collaborator

@Skylion007 Skylion007 commented Oct 7, 2021

Description

  • This PR adds a few pre-commit hooks that will ban a few bad practices in the repo. Mainly ban blanket linter ignores (all ignores need to be error specific and some common RST bugs.
  • I also fixed all the found rst bugs in the repos (exclusively single backticks that should have been double for code).
  • This PR also adds a pre-commit hook that removes all unnecessary NOQAs automatically while keeping the flake8 plugins in sync flake8 hook.
  • I pushed upstream changes to the mypy hook that allows us to simplify our mypy hook config.

Suggested changelog entry:

* Add pre-commit hooks to ban blanket linter ignores 
* Add pre-commit hooks to detect common RST mistakes and fix all found instances of those.
* Add the yesqa pre-commit hook to automatically remove unused noqa comments

@Skylion007 Skylion007 requested review from rwgk and henryiii October 7, 2021 15:10
@henryiii
Copy link
Collaborator

henryiii commented Oct 7, 2021

What are the failures if you add the triple tick check?

@Skylion007
Copy link
Collaborator Author

Skylion007 commented Oct 7, 2021

@henryiii failures from the rst-backticks check

docs/advanced/classes.rst:1269:wish to directly manipulate the `PyHeapTypeObject` corresponding to a
docs/advanced/embedding.rst:43:all the functions and classes in pybind11. The RAII guard class `scoped_interpreter`
docs/advanced/embedding.rst:50:There are a few different ways to run Python code. One option is to use `eval`,
docs/advanced/embedding.rst:51:`exec` or `eval_file`, as explained in :ref:`eval`. Here is a quick example in
docs/advanced/embedding.rst:111:Python modules can be imported using `module_::import()`:
docs/advanced/embedding.rst:137:Modules can be reloaded using `module_::reload()` if the source is modified e.g.
docs/advanced/embedding.rst:147:Embedded binary modules can be added using the `PYBIND11_EMBEDDED_MODULE` macro.
docs/advanced/embedding.rst:173:`PYBIND11_EMBEDDED_MODULE` definitions (as long as they have unique names).
docs/advanced/embedding.rst:219:The Python interpreter shuts down when `scoped_interpreter` is destroyed. After
docs/advanced/embedding.rst:221:`initialize_interpreter` / `finalize_interpreter` pair of functions can be used
docs/advanced/embedding.rst:245:Creating multiple copies of `scoped_interpreter` is not possible because it
docs/advanced/exceptions.rst:99:parameter, a `handle`:
docs/advanced/exceptions.rst:106:Then `PyExp` can be caught both as `PyExp` and `RuntimeError`.
docs/advanced/exceptions.rst:110:The default base class is `PyExc_Exception`.
docs/advanced/functions.rst:235:scope guard will work. This is very useful in combination with `gil_scoped_release`.
docs/advanced/misc.rst:87:The ``call_go`` wrapper can also be simplified using the `call_guard` policy
docs/advanced/pycpp/utilities.rst:31:redirection. Replacing a library's printing with `py::print <print>` may not
docs/advanced/pycpp/utilities.rst:65:extra type, `py::scoped_estream_redirect <scoped_estream_redirect>`, is identical
docs/advanced/pycpp/utilities.rst:67:`py::call_guard`, which allows multiple items, but uses the default constructor:
docs/advanced/pycpp/utilities.rst:77:manager, using the `py::add_ostream_redirect() <add_ostream_redirect>` function:
docs/advanced/pycpp/utilities.rst:106:pybind11 provides the `eval`, `exec` and `eval_file` functions to evaluate
docs/changelog.rst:253:* ``py::str`` changed to exclusively hold `PyUnicodeObject`. Previously
docs/changelog.rst:254:  ``py::str`` could also hold `bytes`, which is probably surprising, was
docs/changelog.rst:1407:* Fixed implicit conversion of `py::enum_` to integer types on Python 2.7.

@Skylion007 Skylion007 changed the title (maint): Add additional pygrep pre-commit hooks (maint): Add additional pre-commit hooks Oct 7, 2021
@Skylion007 Skylion007 changed the title (maint): Add additional pre-commit hooks (maint): Add additional linter-related pre-commit hooks Oct 7, 2021
@henryiii
Copy link
Collaborator

henryiii commented Oct 7, 2021

Pretty sure those are all mistakes, using markdown syntax. If you really intended single-ticks, you should add a role, like

:py:`stuff`

@Skylion007
Copy link
Collaborator Author

@henryiii To clarify, all those errors should be rewritten as by being prepended with :py: ?

@henryiii
Copy link
Collaborator

henryiii commented Oct 7, 2021

No, they should use double tick marks. I think single tick marks use the default role, which is :py:, but I don't think any of those were trying to use it. If it's a fully qualified local name, then using that role is fine. But don't think any of those are, they are just code.

@henryiii henryiii merged commit f4c81e0 into pybind:master Oct 8, 2021
@henryiii henryiii changed the title (maint): Add additional linter-related pre-commit hooks maint: Add additional linter-related pre-commit hooks Oct 8, 2021
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.

2 participants