sourcery-ai / sourcery-vscode

VS Code extension for Sourcery
MIT License
84 stars 11 forks source link

Incorrect suggestion to "merge dictionary updates via the union operator" #139

Closed wardy3 closed 1 year ago

wardy3 commented 1 year ago

Hi

I'm running Python 3.11.2 and now get the above suggestion

While this was fine for the other .update methods I used, one of them is a generator

This causes a TypeError for unsupported operand type generator

Hellebore commented 1 year ago

@wardy3 thanks for raising, and apologies for the delay - have you got a code example where it went wrong?

wardy3 commented 1 year ago

Hi Nick

I just tried to reproduce this and the problem is that it’s not a simple dict

It’s a diskcache.persistent.Index object. So, it acts like a dict but you get an error that it is an unsupported operand for |= between Index and generator

Probably not much you can do about that. I didn’t realise it was a bit of an edge case when I raised this sorry

In case your'e interested ...

"""Try to use union to update with a generator."""

# %%

from collections.abc import Iterator

import diskcache

# %%

def later_on() -> Iterator[tuple[str, str]]:
    """Return a few meals for the afternoon.

    Yields:
        tuple[str,str]: meal name, meal contents
    """
    yield "afternoon snack", "nuts"
    yield "dinner", "veg curry"

# %%

starting = {"breakfast": "eggs", "morning snack": "yoghurt", "lunch": "noodles"}

starting_index: dict[str, str] = diskcache.Index(**starting)

# %%

starting |= later_on()
print(f"{starting=}")

# %%

starting_index |= later_on()  # <--- Sourcery suggests this but it will error out
print(f"{starting_index=}")
starting={'breakfast': 'eggs', 'morning snack': 'yoghurt', 'lunch': 'noodles', 'afternoon snack': 'nuts', 'dinner': 'veg curry'}
Traceback (most recent call last):
  File "/Users/wardy/Documents/EMC/gitlab.nosync/storage/crap.py", line 36, in <module>
    starting_index |= later_on()  # <--- Sourcery suggests this but it will error out
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: unsupported operand type(s) for |=: 'Index' and 'generator'
wardy3 commented 1 year ago

having another look, I think typing it as a dict ruins everything...

Hellebore commented 1 year ago

@wardy3 yeah if it's typed as a dict we assume it is a dict - could you type it as typing.Mapping?

wardy3 commented 1 year ago

I think the library is better typed now and I didn’t specifically type it now. All looks goodThanks for the great extension btw!Sent from my iPhone - Enjoy the brevityOn 14 Apr 2023, at 17:51, Nick Thapen @.***> wrote: @wardy3 yeah if it's typed as a dict we assume it is a dict - could you type it as typing.Mapping?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>