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

[pytest] Reverse PT001 and PT0023 defaults #12106

Merged

Conversation

tjkuson
Copy link
Contributor

@tjkuson tjkuson commented Jun 29, 2024

Summary

This patch inverts the defaults for pytest-fixture-incorrect-parentheses-style (PT001) and pytest-incorrect-mark-parentheses-style (PT003) to prefer dropping superfluous parentheses.

Presently, Ruff defaults to adding superfluous parentheses on pytest mark and fixture decorators for documented purpose of consistency; for example,

 import pytest


-@pytest.mark.foo
+@pytest.mark.foo()
 def test_bar(): ...

This behaviour is counter to the official pytest recommendation and diverges from the flake8-pytest-style plugin as of version 2.0.0 (see m-burst/flake8-pytest-style#272). Seeing as either default satisfies the documented benefit of consistency across a codebase, it makes sense to change the behaviour to be consistent with pytest and the flake8 plugin as well.

This change is breaking, so is gated behind preview (at least under my understanding of Ruff versioning). The implementation of this gating feature is a bit hacky, but seemed to be the least disruptive solution without performing invasive surgery on the #[option()] macro.

Related to #8796.

Caveat

Whilst updating the documentation, I sought to reference the pytest recommendation to drop superfluous parentheses, but couldn't find any official instruction beyond it being a revealed preference within the pytest documentation code examples (as well as the linked issues from a core pytest developer). Thus, the wording of the preference is deliberately timid; it's to cohere with pytest rather than follow an explicit guidance.

Test Plan

cargo nextest run

I also ran

cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT001.py --no-cache --diff --select PT001

and compared against it with --preview to verify that the default does change under preview (I also repeated this with echo '[tool.ruff]\npreview = true' > pyproject.toml to verify that it works with a configuration file).

@tjkuson tjkuson marked this pull request as ready for review June 29, 2024 20:54
The defaults for PT001 and PT003 are counter to the official pytest
recommendation and diverge from the flake8-pytest-style plugin.

This change is breaking so is gated behind preview.
@tjkuson tjkuson force-pushed the fix/invert-flake8-pytest-style branch from c57b159 to 530c084 Compare June 30, 2024 16:03
@MichaReiser MichaReiser added fixes Related to suggested fixes for violations configuration Related to settings and configuration and removed fixes Related to suggested fixes for violations labels Jun 30, 2024
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

This makes sense to me -- thanks.

@charliermarsh charliermarsh added the preview Related to preview mode features label Jul 1, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) July 1, 2024 02:02
@charliermarsh charliermarsh changed the title Reverse PT001 and PT0023 defaults [pytest] Reverse PT001 and PT0023 defaults Jul 1, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) July 1, 2024 02:03
@charliermarsh charliermarsh merged commit d1aeadc into astral-sh:main Jul 1, 2024
18 checks passed
@tjkuson tjkuson deleted the fix/invert-flake8-pytest-style branch July 1, 2024 03:40
dangotbanned added a commit to dangotbanned/altair that referenced this pull request Jul 6, 2024
See astral-sh/ruff#12106

I've fixed to the new behaviour and disabled the rule
weiji14 added a commit to GenericMappingTools/pygmt that referenced this pull request Jul 8, 2024
…3317)

* Enable ruff's literal-membership (PLR6201) rule

Xref https://docs.astral.sh/ruff/rules/literal-membership.

* Fix PLR6201 violations

* Fix PT001 violation due to preview mode setting

Xref https://docs.astral.sh/ruff/settings/#lint_flake8-pytest-style_fixture-parentheses and astral-sh/ruff#12106.

* Check geodataframe column dtype's str name instead of class object
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration preview Related to preview mode features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants