You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/11/19 23:07:23 UTC
[GitHub] [iceberg] rdblue opened a new pull request, #6228: Python: Minor fixes to expression types
rdblue opened a new pull request, #6228:
URL: https://github.com/apache/iceberg/pull/6228
This fixes a couple that #6225 didn't catch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #6228: Python: Minor fixes to expression types
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #6228:
URL: https://github.com/apache/iceberg/pull/6228#discussion_r1027343832
##########
python/pyiceberg/expressions/__init__.py:
##########
@@ -317,7 +317,7 @@ def __eq__(self, other):
return self.term == other.term if isinstance(other, UnboundPredicate) else False
@abstractmethod
- def bind(self, schema: Schema, case_sensitive: bool = True) -> BoundPredicate:
+ def bind(self, schema: Schema, case_sensitive: bool = True) -> BooleanExpression:
Review Comment:
I had to add a type parameter for the bound result to `Unbound` to get types working, but it didn't require many changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko merged pull request #6228: Python: Minor fixes to expression types
Posted by GitBox <gi...@apache.org>.
Fokko merged PR #6228:
URL: https://github.com/apache/iceberg/pull/6228
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #6228: Python: Minor fixes to expression types
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #6228:
URL: https://github.com/apache/iceberg/pull/6228#discussion_r1027156981
##########
python/pyiceberg/expressions/__init__.py:
##########
@@ -317,7 +317,7 @@ def __eq__(self, other):
return self.term == other.term if isinstance(other, UnboundPredicate) else False
@abstractmethod
- def bind(self, schema: Schema, case_sensitive: bool = True) -> BoundPredicate:
+ def bind(self, schema: Schema, case_sensitive: bool = True) -> BooleanExpression:
Review Comment:
@Fokko, this is more accurate because binding can produce constants like `AlwaysTrue()`, but I'm not sure that we want to do it if it messes up type inference after binding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #6228: Python: Minor fixes to expression types
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #6228:
URL: https://github.com/apache/iceberg/pull/6228#discussion_r1027160670
##########
python/pyiceberg/expressions/__init__.py:
##########
@@ -317,7 +317,7 @@ def __eq__(self, other):
return self.term == other.term if isinstance(other, UnboundPredicate) else False
@abstractmethod
- def bind(self, schema: Schema, case_sensitive: bool = True) -> BoundPredicate:
+ def bind(self, schema: Schema, case_sensitive: bool = True) -> BooleanExpression:
Review Comment:
I think that's more accurate as well 👍🏻 We'll lose the bound type indeed, but for me, that's okay. Probably we also want to annotate the `__new__` methods with type annotations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org