roboflow / roboflow-python

The official Roboflow Python package. Manage your datasets, models, and deployments. Roboflow has everything you need to build a computer vision application.
https://docs.roboflow.com/python
Apache License 2.0
271 stars 70 forks source link

chore: Enhance type checking with mypy and improve code quality #285

Closed cdragos closed 1 month ago

cdragos commented 1 month ago

This PR enhances our code quality checks and type safety:

  1. Integrates mypy into our check_code_quality process
  2. Improves path handling using pathlib.Path for better cross-platform support
  3. Fixes several type-related issues caught by mypy
  4. Updates .dockerignore
iurisilvio commented 1 month ago

Why pyright instead of mypy? I don't have strong opinions on which one is better, but I don't like to change tools without good reasons.

I'm happy we now have type checking, but I don't think changing tools will improve it a lot. I prefer to keep iterating on mypy config and enable many options still disabled.

cdragos commented 1 month ago

@iurisilvio Thanks for your feedback. I apologize for not providing a description earlier.

I agree about being cautious with changing tools. While exploring pyright, I found it caught several issues with its default settings that the current setup missed:

One feature I found particularly useful is pyright's Inferred Return Types, similar to TypeScript. Unlike mypy, which marks unannotated return types as Any, pyright provides the actual return type.

Many more differences documented here made me think pyright is a better tool.

To move forward, I propose the following alternatives:

  1. Run both tools in parallel to evaluate which better suits our project needs
  2. Keep mypy and some changes / fixes from my PR

Let me know what you think!

iurisilvio commented 1 month ago

I prefer to not add a new tool now. There is a lot of work improving mypy behaviour and another tool will split efforts with small gains. Can you go with option 2?

cdragos commented 1 month ago

@iurisilvio Thank you for your thorough feedback and guidance throughout this PR process. I've addressed all the comments raised. 🏄🏼

iurisilvio commented 1 month ago

Thanks! 🚢