search

oxan / djangorestframework-dataclasses

Dataclasses serializer for Django REST framework
BSD 3-Clause "New" or "Revised" License
429 stars 28 forks source link

Additional type annotations #8

Closed intgr closed 1 year ago

intgr commented 4 years ago

Maybe I went totally overboard with this :) but I've been searching for a project on which to try all-in mypy type checking support, and this sort of happened.

If you're interested, I can look into adding a mypy check run into Travis CI.

It's not quite there yet. The serializers.py file now wholly validates with mypy (along with djangorestframework-stubs), but not yet other files. typing_utils.py uses a fair bit of magic that (ironically) probably won't ever be understood by mypy, type checking should simply be suppressed there.

oxan commented 4 years ago

I'm a bit torn on this. On one hand I like typechecking and it'd be nice to have that done in CI, but on the other hand I think type declarations on local variables don't really help readability of the code.

However, it seems that most (if not all?) type declarations on local variables are needed because of the getattr(self, 'Meta') things. What do you think about doing that in the constructor (like we already do for dataclass and extra_kwargs) for all options we support? I think that'd be an improvement regardless of the typing.

dineshtrivedi commented 3 years ago

Is there anything I can try to help here, @oxan or @intgr ? I wonder if you are still maintaining this package. I am considering using it.

oxan commented 3 years ago

The library is still maintained.

As for this pull request, I'd still like to make the project mypy-clean, but it's fairly low priority and I don't want to reduce readability of the code. Especially given the dynamic typing hacks we do in certain places that's easier said than done.

intgr commented 3 years ago

Sorry, I got side tracked with other things and sort of forgot about this.

I should give this another shot.

dineshtrivedi commented 3 years ago

Thanks for the response @oxan Awesome, I am happy to help if I can. Let me know if there is anything you could use some help

oxan commented 3 years ago

FYI I've started the mypy branch to (finally) get mypy checking integrated in the CI, but it doesn't pass cleanly yet and I'll probably merge (parts of) this branch into it.

oxan commented 1 year ago

Most of what was in this PR is now merged into master in separate commits. I don't think I'll adopt the remainder soon, enabling strict mypy flags isn't really feasible with all the typing inspection stuff that's going on, and I don't find it helping the readability of the code. Thus, I'll close this PR for now.