strawberry-graphql / strawberry

A GraphQL library for Python that leverages type annotations πŸ“
https://strawberry.rocks
MIT License
3.85k stars 510 forks source link

Enable Ruff annotations checks #2477

Open patrick91 opened 1 year ago

patrick91 commented 1 year ago

In #2476 we enabled a bunch of additional checks on Ruff, but I haven't enabled some that are probably more important for us, namely the ones for annotations. We'd like our library to be fully annotated 😊

We can enable all these checks one at the time:

We can skip these checks on tests, it's nice to have tests type too, but it might take a lot of time 😊

Feel free to ask me any question for this, some of the annotations might be easy to add, some might not 😊

Upvote & Fund

Fund with Polar

patrick91 commented 1 year ago

Annotations are disabled here: https://github.com/strawberry-graphql/strawberry/blob/main/pyproject.toml#L178-L187

To work on any of them, remove the line of the annotation and then run ruff . and see what you get 😊 Then you can work from there to add annotations :)

patrick91 commented 1 year ago

Hi @patrick91, I'm interested in contributing to this issue. Can you please assign this to me

Thank You.

hi @MadhuMPandurangi, I think it's fine to start a PR directly ☺️

Please do only one check at the time, enabling all of them at once will require a lot of work ☺️

alexauritt commented 1 year ago

OK, diving into ANN001. @patrick91 no expectations that you'll want to check a WIP but if you want to take a peek and verify I'm on the right path, might save us some time...

see here

Before:

Screenshot 2023-01-27 at 8 19 49 PM

After:

Screenshot 2023-01-27 at 8 19 40 PM
patrick91 commented 1 year ago

@alexauritt yes, that looks good! Feel free to open a draft PR too if you want more feedback on specific lines 😊

patrick91 commented 1 year ago

@alexauritt it is also fine to have # noqa: ANN001 where it is quite complex to add annotations btw 😊 we can always do this in multiple PRs :)

JadHADDAD92 commented 1 year ago

I wanna work on ANN201, maybe it's better to make it with multiple PRs as there are 1387 errors, what do you think @patrick91

patrick91 commented 1 year ago

@JadHADDAD92 yes, good idea!

alexauritt commented 1 year ago

tackled ANN001 and ANN002

Ambro17 commented 1 year ago

Should i choose any annotation and create a new pr?

brunodantas commented 1 month ago

May want to remove ANN102 from the list, since it is being deprecated.