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

UP031: ruff 0.4.2 generated invalid fix it code #11166

Open
Skylion007 opened this issue Apr 26, 2024 · 8 comments
Open

UP031: ruff 0.4.2 generated invalid fix it code #11166

Skylion007 opened this issue Apr 26, 2024 · 8 comments
Labels
bug Something isn't working fixes Related to suggested fixes for violations

Comments

@Skylion007
Copy link

Skylion007 commented Apr 26, 2024

                msg="\n".join(
                    [
                        "alpha = alpha_c + %.2g" % shift,
                        "expected_grad: %.5g" % expected_grad,
                        "actual_grad: %.5g" % actual_grad,
                        "error = %.2g" % torch.abs(expected_grad - actual_grad).max(),
                    ]

became

                msg="\n".join(
                    [
                        f"alpha = alpha_c + {shift:.2g}",
                        f"expected_grad: {expected_grad:.5g}",
                        f"actual_grad: {actual_grad:.5g}",
                        f"error = {torch.abs(expected_grad - actual_grad).max():.2g}",
                    ]

which throws an error. I know it's an unsafe fix, but unsafe fixes should not generate invalid code. Specifically:

Traceback (most recent call last):
  File "/var/lib/jenkins/workspace/test/distributions/test_distributions.py", line 4658, in test_dirichlet_multivariate
    f"expected_grad: {expected_grad:.5g}",
  File "/opt/conda/envs/py_3.10/lib/python3.10/site-packages/torch/_tensor.py", line 989, in __format__
    return object.__format__(self, format_spec)
TypeError: unsupported format string passed to Tensor.__format__

from this PR: pytorch/pytorch#125031

@AlexWaygood AlexWaygood added bug Something isn't working fixes Related to suggested fixes for violations labels Apr 26, 2024
@AlexWaygood
Copy link
Member

Guessing this is due to the recent change to UP031? Cc. @plredmond

@charliermarsh
Copy link
Member

Why can't tensors be passed-in there, but can be formatted with % formatting?

@Skylion007
Copy link
Author

Skylion007 commented Apr 26, 2024

@charliermarsh I think it's related to classes overriding object.__format__ magic method. Does the '%' method cast it to a string before calling __format__ on it or something?

@Skylion007
Copy link
Author

Skylion007 commented Apr 26, 2024

@charliermarsh Ah, apparently in the first code, it's implicitly converted before: "Here, you're using the old-style string formatting (%-formatting), where shift, a PyTorch tensor, is implicitly converted to a numeric type (float or integer) when it's formatted. This conversion happens before the formatting is applied, so the process doesn't raise any issues."

import torch
x = torch.tensor([1.0])
print("Value: %.5g" % x)  # Works because x is cast as a float before __format__ is called
print(f"Value: {x:.5g}") # fails because Tensor.__format__ is called

@Skylion007 Skylion007 changed the title Ruff: 0.4.2 generated invalid fix it code UP031: ruff 0.4.2 generated invalid fix it code Apr 26, 2024
@Skylion007
Copy link
Author

Skylion007 commented Apr 28, 2024

I think explicitly casting to float in all float specific f-string fixits could solve this issue?

@charliermarsh
Copy link
Member

I think that makes sense. E.g., when there's a :.5g specifier?

@Skylion007
Copy link
Author

There are probably other specifies that do this casting f (or maybe some to int for d) etc.

@plredmond
Copy link
Contributor

@charliermarsh perhaps we could add this kind of explicit-conversion to the UP031 rule after we can infer the types of the expressions? alternatively, since we parse the format string we could use that information to select an explicit-conversion function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixes Related to suggested fixes for violations
Projects
None yet
Development

No branches or pull requests

4 participants