JelleZijlstra / autotyping

Automatically add simple type annotations to your code
230 stars 19 forks source link

Do not add None for abstractmethods #33

Closed twoertwein closed 2 years ago

twoertwein commented 2 years ago

and not in pyi files.

Could also be a "features" to force people annotate all abstract methods (and all functions in pyi files).

JelleZijlstra commented 2 years ago

and not in pyi files.

Could you clarify this? I don't think it makes sense to run autotyping on a pyi.

Could also be a "features" to force people annotate all abstract methods (and all functions in pyi files).

There are other tools to force annotations, that's not what autotyping is for :)

JelleZijlstra commented 2 years ago

and not in pyi files.

Could you clarify this? I don't think it makes sense to run autotyping on a pyi.

Actually, looking at your linked pandas PR I see you got it to add None return types to __init__ methods in stubs, which seems useful enough. I'll try to make it exclude stubs too for the None return fix.

twoertwein commented 2 years ago

about stubs files: I simply appended --include-stubs as suggested by the --help documentation

twoertwein commented 2 years ago

and not in pyi files.

Could you clarify this? I don't think it makes sense to run autotyping on a pyi.

Actually, looking at your linked pandas PR I see you got it to add None return types to __init__ methods in stubs, which seems useful enough. I'll try to make it exclude stubs too for the None return fix.

There was some re-formatting when running it on pyi files. I assume the way autotyping (or libcst) calls black is different from how it is configured in pandas: it inserted blank lines between class variables and methods.

JelleZijlstra commented 2 years ago

and not in pyi files.

Could you clarify this? I don't think it makes sense to run autotyping on a pyi.

Actually, looking at your linked pandas PR I see you got it to add None return types to __init__ methods in stubs, which seems useful enough. I'll try to make it exclude stubs too for the None return fix.

There was some re-formatting when running it on pyi files. I assume the way autotyping (or libcst) calls black is different from how it is configured in pandas: it inserted blank lines between class variables and methods.

Black uses a different formatting mode for stubs. LibCST passes the code to be formatted through stdin, so Black doesn't know it's a stub. Not sure this is easily fixable.

Anyway, I am preparing a fix for this issue. Let me know if there's anything else you'd like changed; I can cut a release after this issue is fixed so you can use it in pandas.

twoertwein commented 2 years ago

Thank you @JelleZijlstra!

I'm new to LibCST, I saw there is an option in its code to do no formatting (maybe that helps with the pyi files). Can this be set through the config file or when calling autotyping?

JelleZijlstra commented 2 years ago

I believe if you remove the formatter line from .libcst.codemod.yaml (https://github.com/JelleZijlstra/autotyping/blob/master/.libcst.codemod.yaml#L7), LibCST won't run formatting automatically. That might mean you have to call Black manually on some .py files though.

twoertwein commented 2 years ago

Thank you for the quick fix :)

It seems that black is part of the default formatter, when removing the line formatter it still runs black. When providing an empty list, it raises Exception("No formatter configured but code formatting requested.").

Stupid, but it works (at least on linux): formatter: ["cat"]

twoertwein commented 2 years ago

Probably easier to simply install black as part of the pre-commit step. If there are no changes, black will not be called on pyi files.