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/15 14:29:55 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #6197: Python: Fix rough edges around literals
Fokko opened a new pull request, #6197:
URL: https://github.com/apache/iceberg/pull/6197
From https://github.com/apache/iceberg/pull/6141, for follow-up:
> Fix types returned by to when AboveMax/BelowMin are returned
Fixed
- Add tests for binding with invalid conversions
Added some tests to cover the last few uncovered lines:
```
poetry run coverage report -m --fail-under=90
Name Stmts Miss Cover Missing
------------------------------------------------------------------------
...
pyiceberg/expressions/literals.py 331 0 100%
```
- Update binding to rewrite expressions when AboveMax and BelowMin are returned
First thing after https://github.com/apache/iceberg/pull/6139 has been merged
- Check whether we need AboveMax/BelowMin for longs, since Python uses arbitrary precision numbers
Do you mean when you run Python on a 32bit computer? This will reduce the precision.
- Improve some of the error messages
Went over all of them 👍🏻
--
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 pull request #6197: Python: Fix rough edges around literals
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #6197:
URL: https://github.com/apache/iceberg/pull/6197#issuecomment-1317275633
> Do you mean when you run Python on a 32bit computer? This will reduce the precision.
What I mean is that Python `int` can contain values that are larger than 64 bits because it uses arbitrary precision. If we get `int` values in expressions that are larger than a 64-bit integer then we should do the same thing as for the 32-bit integer and rewrite the expression to `AlwaysTrue()` or `AlwaysFalse()`.
--
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 #6197: Python: Fix rough edges around literals
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #6197:
URL: https://github.com/apache/iceberg/pull/6197#discussion_r1024484631
##########
python/pyiceberg/utils/datetime.py:
##########
@@ -81,13 +81,19 @@ def timestamp_to_micros(timestamp_str: str) -> int:
"""Converts an ISO-9601 formatted timestamp without zone to microseconds from 1970-01-01T00:00:00.000000"""
if ISO_TIMESTAMP.fullmatch(timestamp_str):
return datetime_to_micros(datetime.fromisoformat(timestamp_str))
+ if ISO_TIMESTAMPTZ.fullmatch(timestamp_str):
+ # When we can match a timestamp without a zone, we can give a more specific error
+ raise ValueError(f"Timezone provided, but not expected: {timestamp_str}")
raise ValueError(f"Invalid timestamp without zone: {timestamp_str} (must be ISO-8601)")
def timestamptz_to_micros(timestamptz_str: str) -> int:
"""Converts an ISO-8601 formatted timestamp with zone to microseconds from 1970-01-01T00:00:00.000000+00:00"""
if ISO_TIMESTAMPTZ.fullmatch(timestamptz_str):
return datetime_to_micros(datetime.fromisoformat(timestamptz_str))
+ if ISO_TIMESTAMP.fullmatch(timestamptz_str):
+ # When we can match a timestamp without a zone, we can give a more specific error
+ raise ValueError(f"Missing timezone: {timestamptz_str} (must be ISO-8601)")
Review Comment:
Ah, wasn't aware of that! Thanks!
--
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 #6197: Python: Fix rough edges around literals
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #6197:
URL: https://github.com/apache/iceberg/pull/6197#discussion_r1024213544
##########
python/pyiceberg/utils/datetime.py:
##########
@@ -81,13 +81,19 @@ def timestamp_to_micros(timestamp_str: str) -> int:
"""Converts an ISO-9601 formatted timestamp without zone to microseconds from 1970-01-01T00:00:00.000000"""
if ISO_TIMESTAMP.fullmatch(timestamp_str):
return datetime_to_micros(datetime.fromisoformat(timestamp_str))
+ if ISO_TIMESTAMPTZ.fullmatch(timestamp_str):
+ # When we can match a timestamp without a zone, we can give a more specific error
+ raise ValueError(f"Timezone provided, but not expected: {timestamp_str}")
raise ValueError(f"Invalid timestamp without zone: {timestamp_str} (must be ISO-8601)")
def timestamptz_to_micros(timestamptz_str: str) -> int:
"""Converts an ISO-8601 formatted timestamp with zone to microseconds from 1970-01-01T00:00:00.000000+00:00"""
if ISO_TIMESTAMPTZ.fullmatch(timestamptz_str):
return datetime_to_micros(datetime.fromisoformat(timestamptz_str))
+ if ISO_TIMESTAMP.fullmatch(timestamptz_str):
+ # When we can match a timestamp without a zone, we can give a more specific error
+ raise ValueError(f"Missing timezone: {timestamptz_str} (must be ISO-8601)")
Review Comment:
This should be more specific that the problem is that the string is missing a "zone offset". We don't accept time zone names.
##########
python/pyiceberg/utils/datetime.py:
##########
@@ -81,13 +81,19 @@ def timestamp_to_micros(timestamp_str: str) -> int:
"""Converts an ISO-9601 formatted timestamp without zone to microseconds from 1970-01-01T00:00:00.000000"""
if ISO_TIMESTAMP.fullmatch(timestamp_str):
return datetime_to_micros(datetime.fromisoformat(timestamp_str))
+ if ISO_TIMESTAMPTZ.fullmatch(timestamp_str):
+ # When we can match a timestamp without a zone, we can give a more specific error
+ raise ValueError(f"Timezone provided, but not expected: {timestamp_str}")
Review Comment:
Zone offset here as well.
--
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 #6197: Python: Fix rough edges around literals
Posted by GitBox <gi...@apache.org>.
Fokko merged PR #6197:
URL: https://github.com/apache/iceberg/pull/6197
--
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