Skip to content

Conversation

@uf-fipper
Copy link

fix type hint

utils.py

  • fix python 3.6.12 support
  • fix builder overload
  • add ignore_copy type hint
  • fix validate type hint

terms.py

  • add wrap_constant overload
  • change wrap_constant value type: Node -> Term
  • add wrap_json overload
  • remove Term._assert_guard
  • add get_formatted_value return type
  • add python3.6, 3.7, 3.8, 3.9 support (Case.__init__)
  • change WindowFrameAnalyticFunction.Edge.modifier default value
  • fix other type
  • Term.any Term.all: fix return type

queries.py

  • Selectable: change Base Type: Node -> Term
  • Table: add generic type
  • Query: add generic type
  • __copy__: return Self type
  • remove Term._assert_guard
  • Joiner: add generic type
  • fix other type

dialects.py

  • add Query generic
  • __copy__: return Self type
  • change classmethod first argument: self -> cls
  • fix bug: in queries.make_tables

array.py search_string.py type_conversion.py

  • fix Optional and Schema type

about generic

a = Table("a")
b = Table("b")

q1 = Query.from_(a).select().left_join(b).on(a.f == b.f).where(a.f == 1).orderby(a.f1)  # q1 is QueryBuilder
q2 = OracleQuery.from_(a).select().left_join(b).on(a.f == b.f).where(a.f == 1).orderby(a.f1)  # q2 is OracleQueryBuilder
a = Query.Table("a").select("*")  # a is QueryBuilder
b = OracleQuery.Table("b").select("*")  # b is OracleQueryBuilder
c = Table("c", query_cls=ClickHouseQuery).update()  # c is ClickHouseQueryBuilder
d = Table("d", query_cls=SQLLiteQuery).insert()  # d is SQLLiteQueryBuilder

@thislight
Copy link
Owner

Hello uf-fipper, I am happy to review (and merge) your changes in the following days. Ping me if you want to add more commits.

@thislight thislight self-assigned this Feb 27, 2023
@thislight thislight changed the title Fix type hint2 Fix more type hint Feb 27, 2023
@thislight thislight self-requested a review February 27, 2023 12:08
@uf-fipper
Copy link
Author

Thanks for your reply! I am looking forward to our cooperation.

fish and others added 16 commits February 27, 2023 22:31
fix python3.6 support
fix builder overload
add ignore_copy type hint
fix validate type hint
fix python3.6 support
fix builder overload
add ignore_copy type hint
fix validate type hint
add wrap_constant's overload
change wrap_constant's value type: Node -> Term
add wrap_json's overload
remove Term._assert_guard
add get_formatted_value's return type
add python3.6, 3.7, 3.8, 3.9 support (Case.__init__)
change WindowFrameAnalyticFunction.Edge.modifier default value
fix other type
add wrap_constant's overload
change wrap_constant's value type: Node -> Term
add wrap_json's overload
remove Term._assert_guard
add get_formatted_value's return type
add python3.6, 3.7, 3.8, 3.9 support (Case.__init__)
change WindowFrameAnalyticFunction.Edge.modifier default value
fix other type
Selectable: change Base Type: Node -> Term
Table: add generic type
Query: add generic type
__copy__: return Self type
remove Term._assert_guard
Joiner: add generic type
fix other type
Selectable: change Base Type: Node -> Term
Table: add generic type
Query: add generic type
__copy__: return Self type
remove Term._assert_guard
Joiner: add generic type
fix other type
add Query generic
__copy__: return Self type
change classmethod first argument: self -> cls
fix bug: in queries.make_tables
add Query generic
__copy__: return Self type
change classmethod first argument: self -> cls
fix bug: in queries.make_tables
@thislight
Copy link
Owner

thislight commented Feb 27, 2023

uf-fipper, would you like to rewrite all your commits for a different email address? Looks like it is not asscoicated to any account on GitHub.

Also: I had been added an automated test for type checking, so I rebase commits for you.

Copy link
Owner

@thislight thislight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine for me, but your commits are commited by fish <fish@fish.fish>. Would you like to rewrite them to your own identity?

Also: you may run black on your code, so we can fix the format problem.

@uf-fipper
Copy link
Author

I changed my email in the latest commit, and I reformat file using black.

@thislight thislight self-requested a review March 1, 2023 01:35
...

@staticmethod
def wrap_json(
Copy link
Owner

@thislight thislight Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrap_json()'s overloads are reported by mypy:

  • Overloaded function signatures 1 and 6 overlap with incompatible return types [misc]
  • Overloaded function signature 2 will never be matched: signature 1's parameter type(s) are the same or broader [misc]
    • (Rubicon: This signature might be removed since the TermT is including IntervalT, Interval is already a Term)
  • Overloaded function signatures 2 and 6 overlap with incompatible return types [misc]
  • Overloaded function signatures 3 and 6 overlap with incompatible return types [misc]
  • Overloaded function signatures 4 and 6 overlap with incompatible return types [misc]
  • Overloaded function signatures 5 and 6 overlap with incompatible return types [misc]

Copy link
Owner

@thislight thislight Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest errors are looked like they could not be solved with a workaround, may you leave a comment there as a tip?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used mypy to check my code, but I found more issues such as generics, and the "default" parameter in TypeVar. I need to look into these issues over the next few days. Before that, I used pyright (basic mode) to check my code. I'm sorry about that.

@uf-fipper
Copy link
Author

Hi thislight, I used mypy to recheck the code and added some type ignore. However, during the test, I found another problem: in the version of python 3.8+, use pip install - r requirements-dev.txt to install dependencies, there is no typing_extensions. So I modified the code that partially depends on typing_extensions.
I changed the base class of Selectable into Node and found two problems that I could not solve. I annotated those two places with # TODO.

@thislight thislight self-requested a review March 16, 2023 07:48
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.

2 participants