Skip to content

Conversation

DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Jun 18, 2025

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.rst
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Jun 18, 2025
@dstansby
Copy link
Contributor

What do these rules do? Can we fix for them instead of ignoring them?

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Jun 18, 2025

We could, but some maintainers don't like them:

PT011 `pytest.raises(ValueError)` is too broad, set the `match` parameter or use a more specific exception
PT012 `pytest.raises()` block should contain a single simple statement
PT030 `pytest.warns(DeprecationWarning)` is too broad, set the `match` parameter or use a more specific warning
PT031 `pytest.warns()` block should contain a single simple statement

@DimitriPapadopoulos DimitriPapadopoulos force-pushed the ruff_0.12 branch 2 times, most recently from 674c042 to 222ba71 Compare July 1, 2025 22:01
@dstansby
Copy link
Contributor

dstansby commented Jul 6, 2025

In my opinion:

PT011 pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
PT030 pytest.warns(DeprecationWarning) is too broad, set the match parameter or use a more specific warning

We should definitely enforce these.

PT012 pytest.raises() block should contain a single simple statement
PT031 pytest.warns() block should contain a single simple statement

I'd be interested to see what needs fixing to enable these, and if it's reasonable changes enforce them.

@dstansby
Copy link
Contributor

dstansby commented Jul 6, 2025

If you'd be up for it, a PR to fix the first two (adding match to pytest.raises) would be great, and a separate PR to see what fixing the second two would be good too.

@DimitriPapadopoulos
Copy link
Contributor Author

I'd rather avoid adding the match argument in this PR, changes are extensive as far as I can remember.

As for the second two, they imply transforming large pytest.raises()/pytest.warns() blocks into a small block that contains only the statement that raises or emits the warning. The rest being is moved outside the block.

@DimitriPapadopoulos
Copy link
Contributor Author

Superseded by #3214.

@DimitriPapadopoulos DimitriPapadopoulos deleted the ruff_0.12 branch July 7, 2025 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs release notes Automatically applied to PRs which haven't added release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants