glutanimate / review-heatmap

Anki add-on to help you keep track of your review activity
https://ankiweb.net/shared/info/1771074083
Other
1.2k stars 89 forks source link

Removed workaround for list comprehensions #132

Closed NeTiFeSi closed 2 years ago

NeTiFeSi commented 2 years ago

Removed workaround for list comprehensions not working in class-scope

Checklist:

Please replace the space inside the brackets with an x and fill out the ellipses if the following items apply:

glutanimate commented 2 years ago

Hey Felipe, sorry for taking ages to get to this PR!

I think this solution will not work either, unfortunately, at least not without adjustment. It seems like css_colors is not defined in the lambda scope, so you end up with:

    map(lambda i: css_colors[i], (0, 2, 4, 6, 9, 10)),
NameError: name 'css_colors' is not defined

This can be fixed by capturing css_colors early like this:

map(lambda index, colors=css_colors: colors[index], (0, 2, 4, 6, 9, 10)),

but mypy doesn't seem to particularly like lambda functions, so you end up with:

Cannot infer type of lambda  [misc]mypy(error)

and once you tag on a # type: ignore, I feel like the statement becomes just as unwieldy as the current solution (and slightly less type-safe).

So I think we should stick with the current solution for now, even though I like your approach more. I am planning on refactoring how CSS works across the add-on soon, so maybe this jankiness will just go a way as a side-effect of that.

In any case, thanks a bunch for you contribution, it's much appreciated!

NeTiFeSi commented 2 years ago

No problem, thank you!