ruff icon indicating copy to clipboard operation
ruff copied to clipboard

Undesirable invalid property name error in convert_typed_dict_functional_to_class

Open martinlehoux opened this issue 2 years ago • 3 comments

Just adding a reminder, I'll try to get to it soon

When running pyupgrade on my codebase, I got the following error:

[2022-12-12][16:54:45][ruff::pyupgrade::plugins::convert_typed_dict_functional_to_class][ERROR] Failed to parse TypedDict: Invalid property name: xsi:noNamespaceSchemaLocation

I remember we logged an error because we couldn't upgrade it, but I shouldn't see an error as a user: I know that I can convert this TypedDict to class syntax, because I have weird keys

martinlehoux avatar Dec 12 '22 15:12 martinlehoux

Or the user based solution is to ignore this specific TypedDict?

martinlehoux avatar Dec 12 '22 15:12 martinlehoux

I think we could flag the error, but not try to upgrade it?

charliermarsh avatar Dec 20 '22 01:12 charliermarsh

It all depends on what we expect the user to do: do they have to explicitly ignore the error ? I guess that would be the simplest

martinlehoux avatar Dec 23 '22 21:12 martinlehoux

Assuming I understand the above, it seems the current Ruff behavior here is still not quite desirable. Per the python official docs here:

The functional syntax should also be used when any of the keys are not valid identifiers, for example because they are keywords or contain hyphens. Example:

# raises SyntaxError
class Point2D(TypedDict):
    in: int  # 'in' is a keyword
    x-y: int  # name with hyphens

# OK, functional syntax
Point2D = TypedDict('Point2D', {'in': int, 'x-y': int})

As such, Ruff's current behavior of showing a violation of "UP013 Convert Point2D from TypedDict functional to class syntax" in the above functional syntax example seems undesirable, since it's actually the recommended/only way to use TypedDict. Rather, Ruff should just silently ignore. (pyupgrade correctly ignores this scenario.) Should I file a new issue for this, or should this one be reopened potentially?

Thank you for this awesome library!

sjdemartini avatar Feb 22 '23 22:02 sjdemartini

Ah yeah, I agree with you. Can you create a new issue? I'll fix it real quick.

charliermarsh avatar Feb 22 '23 22:02 charliermarsh

Yes absolutely, thanks for the quick help! Here's an issue https://github.com/charliermarsh/ruff/issues/3147. Your speed on GitHub rivals the speed of Ruff itself haha

sjdemartini avatar Feb 22 '23 22:02 sjdemartini

😅

charliermarsh avatar Feb 22 '23 23:02 charliermarsh