fix: Typing hint for builder decorator

kayak / pypika

PyPika is a python SQL query builder that exposes the full richness of the SQL language using a syntax that reflects the resulting query. PyPika excels at all sorts of SQL queries but is especially useful for data analysis.

http://pypika.readthedocs.io/en/latest/

Apache License 2.0

2.43k stars 293 forks source link

fix: Typing hint for builder decorator #740

Closed gavindsouza closed 9 months ago

gavindsouza commented 11 months ago

Maintain that the Callable type received is the same as the Callable returned.

Additional Context for the screenshots: tbl is a Table object, frappe.qb is a namespace that has type MySQLQuery | PostgreSQLQuery

Before

before-typing-change

After

after-typing-change

AzisK commented 10 months ago

Could you elaborate on this change a little bit more? I don't fully get it

gavindsouza commented 10 months ago

@AzisK The previous type hints suggested that the builder function took a Callable input and returned a Callable but they need not be a Callable of the same type. This would confuse the type checkers while evaluating method chains, as you can see from the unhighlighted recurring where and delete chains in the "Before" section.

The change I'm proposing says that the Callable returned is that of the same type as the input received - which is of a query builder type that implements the accessed methods. Type checkers understand this and the consequence is that your IDE provides appropriate type hints as highlighted in the "After" section.

The example is usage from Frappe's codebase.

AzisK commented 10 months ago

Interesting. Okay, thanks for the explanation as well as for your work

AzisK commented 10 months ago

We need to lint the code as well as fix the unit test runners

gavindsouza commented 10 months ago

@AzisK We should add a pre-commit config that "black"en's the codebase automatically by the way.

As for the failing 3.6 build, it's high time we drop it given that it's unmaintained and there are 6 new major versions released meanwhile - https://github.com/kayak/pypika/issues/747

AzisK commented 10 months ago

I agree about the pre-commit hook. I just created an issue https://github.com/kayak/pypika/issues/748

I believe we can drop support for Python3.6, I am just not sure what to do with the version of the library. Do we have to bump the major version in this case?

AzisK commented 9 months ago

@gavindsouza could you update this PR with the newest CI code from master?

coveralls commented 9 months ago

coverage: 98.423%. remained the same when pulling cc2d25b4b655fdb5d657ccdb6d9c615a9ffcea55 on gavindsouza:fix-typing-builders into a1b0c82e9025cdd33b3d7aaf5f72202c3afd167e on kayak:master.

gavindsouza commented 9 months ago

@gavindsouza could you update this PR with the newest CI code from master?

Done 😄

AzisK commented 9 months ago

Good job. I am merging this but new version and release will be coming later