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

Ruff 0.3.5 not respecting directive for all lines of a multiline expression #10795

Closed
BenZickel opened this issue Apr 5, 2024 · 6 comments · Fixed by #10798
Closed

Ruff 0.3.5 not respecting directive for all lines of a multiline expression #10795

BenZickel opened this issue Apr 5, 2024 · 6 comments · Fixed by #10798

Comments

@BenZickel
Copy link

The bug occurs when switching from ruff 0.3.4 to 0.3.5 with the directive # noga: F822 not respected for all lines of a multiline expression.

The problem is described in this issue in the Pyro repo, mainly:

  • Linting with ruff version 0.3.5 fails as can be seen here.
  • The failure is due to ruff version 0.3.5 not respecting this directive.
  • The problem does not occur with ruff version 0.3.4.

The command that is executed is ruff check . and is part of the CI workflow of the Pyro repo.

@charliermarsh
Copy link
Member

Hmm... I don't think we respected # noqa in that position in 0.3.4. Is it possible that we weren't raising diagnostics there at all in 0.3.4? I'll test it out.

@charliermarsh
Copy link
Member

Nevermind, I stand corrected! I'll take a look at what changed...

@charliermarsh
Copy link
Member

Ah ok, the change is that we fixed the ranges on those diagnostics to be on the actual strings. So, previously, they all said line 350:

pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Bernoulli` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Cauchy` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Chi2` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `ContinuousBernoulli` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `ExponentialFamily` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Exponential` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `FisherSnedecor` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Gumbel` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `HalfCauchy` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `HalfNormal` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Kumaraswamy` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Laplace` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `LKJCholesky` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `LogisticNormal` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `MixtureSameFamily` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `NegativeBinomial` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `OneHotCategoricalStraightThrough` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Pareto` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `RelaxedBernoulli` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `RelaxedOneHotCategorical` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `StudentT` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `TransformedDistribution` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `VonMises` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Weibull` in `__all__`
pyro/pyro/distributions/torch.py:350:1: F822 Undefined name `Wishart` in `__all__`

Now, we have the correct lines and columns:

pyro/pyro/distributions/torch.py:351:5: F822 Undefined name `Bernoulli` in `__all__`
pyro/pyro/distributions/torch.py:355:5: F822 Undefined name `Cauchy` in `__all__`
pyro/pyro/distributions/torch.py:356:5: F822 Undefined name `Chi2` in `__all__`
pyro/pyro/distributions/torch.py:357:5: F822 Undefined name `ContinuousBernoulli` in `__all__`
pyro/pyro/distributions/torch.py:359:5: F822 Undefined name `ExponentialFamily` in `__all__`
pyro/pyro/distributions/torch.py:360:5: F822 Undefined name `Exponential` in `__all__`
pyro/pyro/distributions/torch.py:361:5: F822 Undefined name `FisherSnedecor` in `__all__`
pyro/pyro/distributions/torch.py:364:5: F822 Undefined name `Gumbel` in `__all__`
pyro/pyro/distributions/torch.py:365:5: F822 Undefined name `HalfCauchy` in `__all__`
pyro/pyro/distributions/torch.py:366:5: F822 Undefined name `HalfNormal` in `__all__`
pyro/pyro/distributions/torch.py:368:5: F822 Undefined name `Kumaraswamy` in `__all__`
pyro/pyro/distributions/torch.py:369:5: F822 Undefined name `Laplace` in `__all__`
pyro/pyro/distributions/torch.py:370:5: F822 Undefined name `LKJCholesky` in `__all__`
pyro/pyro/distributions/torch.py:372:5: F822 Undefined name `LogisticNormal` in `__all__`
pyro/pyro/distributions/torch.py:374:5: F822 Undefined name `MixtureSameFamily` in `__all__`
pyro/pyro/distributions/torch.py:377:5: F822 Undefined name `NegativeBinomial` in `__all__`
pyro/pyro/distributions/torch.py:380:5: F822 Undefined name `OneHotCategoricalStraightThrough` in `__all__`
pyro/pyro/distributions/torch.py:381:5: F822 Undefined name `Pareto` in `__all__`
pyro/pyro/distributions/torch.py:383:5: F822 Undefined name `RelaxedBernoulli` in `__all__`
pyro/pyro/distributions/torch.py:384:5: F822 Undefined name `RelaxedOneHotCategorical` in `__all__`
pyro/pyro/distributions/torch.py:385:5: F822 Undefined name `StudentT` in `__all__`
pyro/pyro/distributions/torch.py:386:5: F822 Undefined name `TransformedDistribution` in `__all__`
pyro/pyro/distributions/torch.py:388:5: F822 Undefined name `VonMises` in `__all__`
pyro/pyro/distributions/torch.py:389:5: F822 Undefined name `Weibull` in `__all__`
pyro/pyro/distributions/torch.py:390:5: F822 Undefined name `Wishart` in `__all__`

But now your # noqa no longer works, since it's on the "wrong" line.

@BenZickel
Copy link
Author

Thanks for the quick reply @charliermarsh!
So it was never the intention to support applying a single directive to all lines of a multiline expression?

@charliermarsh
Copy link
Member

It wasn't the intention, but we can support it I think.

@charliermarsh
Copy link
Member

Will be fixed in the next release: #10798. Thanks, and sorry for the disruption.

charliermarsh added a commit that referenced this issue Apr 6, 2024
## Summary

Historically, given:

```python
__all__ = [  # noqa: F822
    "Bernoulli",
    "Beta",
    "Binomial",
]
```

The F822 violations would be attached to the `__all__`, so this `# noqa`
would be enforced for _all_ definitions in the list. This changed in
#10525 for the better, in that we
now use the range of each string. But these `# noqa` directives stopped
working.

This PR sets the `__all__` as a parent range in the diagnostic, so that
these directives are respected once again.

Closes #10795.

## Test Plan

`cargo test`
BenZickel pushed a commit to BenZickel/pyro that referenced this issue Apr 12, 2024
fritzo pushed a commit to pyro-ppl/pyro that referenced this issue Apr 12, 2024
Co-authored-by: Ben Zickel <ben@vesttoo.com>
Glyphack pushed a commit to Glyphack/ruff that referenced this issue Apr 12, 2024
## Summary

Historically, given:

```python
__all__ = [  # noqa: F822
    "Bernoulli",
    "Beta",
    "Binomial",
]
```

The F822 violations would be attached to the `__all__`, so this `# noqa`
would be enforced for _all_ definitions in the list. This changed in
astral-sh#10525 for the better, in that we
now use the range of each string. But these `# noqa` directives stopped
working.

This PR sets the `__all__` as a parent range in the diagnostic, so that
these directives are respected once again.

Closes astral-sh#10795.

## Test Plan

`cargo test`
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 a pull request may close this issue.

2 participants