Skip to content

Conversation

@thislight
Copy link

@thislight thislight commented Aug 6, 2022

This pull request is intended to revised existing type hint in this project and fix all problems about typing reported by mypy.

Including fixes:

  • __eq__() and __ne__() accept any object as the argument.
  • new type comment for decorator builder
  • improve type comments
  • fix errors in terms.py, utils.py, dialects.py

TODO:

  • find workarounds for old python (pypika requires python 3.6+)

@thislight thislight changed the title Fix Type Hint Fix type hint Aug 6, 2022
@thislight
Copy link
Author

I discover a failed test for terms.py:

ERROR: test__criterion_replace_table_with_value (pypika.tests.test_internals.TablesTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "~/Project/pypika/pypika/tests/test_internals.py", line 16, in test__criterion_replace_table_with_value
    self.assertEqual(c.left, table)
  File "/usr/lib64/pypy3.9/unittest/case.py", line 837, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib64/pypy3.9/unittest/case.py", line 827, in _baseAssertEqual
    if not first == second:
  File "~/Project/pypika/pypika/terms.py", line 265, in __eq__
    return BasicCriterion(Equality.eq, self, Term._assert_guard(self.wrap_constant(other)))
  File "~/Project/pypika/pypika/terms.py", line 301, in _assert_guard
    raise TypeError("expect Term object, got {}".format(type(v).__name__))
TypeError: expect Term object, got Table

I wonder if the original behaviour in the test is intended.

thislight and others added 10 commits August 6, 2022 23:08
- fix criterion_replace_table_with_value tests Field == Table, which
  is unintended.
- fix mypy errors in queries.py
- add Protocol class SQLPart
* Added type checking CI flow

* ci/typechecking: Removed parallel limit

* ci/typechecking: Limit triggers

* ci/typechecking: Set trigger to push

* ci/typechecking: fixed CI using python 3.1 instead of 3.10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant