You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "MehulBatra (via GitHub)" <gi...@apache.org> on 2024/01/03 10:32:51 UTC
[PR] Bug fix falsy value of zero [iceberg-python]
MehulBatra opened a new pull request, #249:
URL: https://github.com/apache/iceberg-python/pull/249
Resolves: [](https://github.com/apache/iceberg-python/issues/232)
Python, certain values are considered False in a boolean context. These include None, 0, empty sequences/collections (`''`, `[]`, `{}`), and others. This is often referred to as “falsy” values.
With the current implementation of ->
`def current_snapshot(self) -> Optional[Snapshot]
`
it will skip snapshot_id = 0 as well, hence to solve this raise this PR
--
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
Re: [PR] Bug fix falsy value of zero [iceberg-python]
Posted by "MehulBatra (via GitHub)" <gi...@apache.org>.
MehulBatra commented on PR #249:
URL: https://github.com/apache/iceberg-python/pull/249#issuecomment-1875156254
Tested Locally all three seems to be passing
```
class DummyClass:
def __init__(self, metadata):
self.metadata = metadata
def test_snapshot_id_condition():
import unittest
class TestMyClass(unittest.TestCase):
def test_snapshot_id_truthy(self):
instance = DummyClass(metadata={'current_snapshot_id': 42})
self.assertTrue((snapshot_id := instance.metadata.get('current_snapshot_id')) is not None or isinstance(snapshot_id, int))
def test_snapshot_id_falsy(self):
instance = DummyClass(metadata={'current_snapshot_id': None})
self.assertFalse((snapshot_id := instance.metadata.get('current_snapshot_id')) is not None or isinstance(snapshot_id, int))
def test_snapshot_id_zero(self):
instance = DummyClass(metadata={'current_snapshot_id': 0})
self.assertTrue((snapshot_id := instance.metadata.get('current_snapshot_id')) is not None or isinstance(snapshot_id, int))
return unittest.TestLoader().loadTestsFromTestCase(TestMyClass)
if __name__ == '__main__':
unittest.TextTestRunner().run(test_snapshot_id_condition())
```
<img width="692" alt="image" src="https://github.com/apache/iceberg-python/assets/66407733/eb36d997-1b03-44be-9004-ba8dc8d6d4da">
--
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
Re: [PR] Bug fix falsy value of zero [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #249:
URL: https://github.com/apache/iceberg-python/pull/249
--
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
Re: [PR] Bug fix falsy value of zero [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #249:
URL: https://github.com/apache/iceberg-python/pull/249#discussion_r1440884348
##########
pyiceberg/table/__init__.py:
##########
@@ -545,7 +545,7 @@ def new_snapshot_id(self) -> int:
def current_snapshot(self) -> Optional[Snapshot]:
"""Get the current snapshot for this table, or None if there is no current snapshot."""
- if snapshot_id := self.metadata.current_snapshot_id:
+ if (snapshot_id := self.metadata.current_snapshot_id) is not None or isinstance(snapshot_id, int):
return self.snapshot_by_id(snapshot_id)
Review Comment:
I think the following is easier to read, WDYT?
```suggestion
if self.metadata.current_snapshot_id is not None:
return self.snapshot_by_id(self.metadata.current_snapshot_id)
```
--
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
Re: [PR] Bug fix falsy value of zero [iceberg-python]
Posted by "MehulBatra (via GitHub)" <gi...@apache.org>.
MehulBatra commented on code in PR #249:
URL: https://github.com/apache/iceberg-python/pull/249#discussion_r1441371562
##########
pyiceberg/table/__init__.py:
##########
@@ -545,7 +545,7 @@ def new_snapshot_id(self) -> int:
def current_snapshot(self) -> Optional[Snapshot]:
"""Get the current snapshot for this table, or None if there is no current snapshot."""
- if snapshot_id := self.metadata.current_snapshot_id:
+ if (snapshot_id := self.metadata.current_snapshot_id) is not None or isinstance(snapshot_id, int):
return self.snapshot_by_id(snapshot_id)
Review Comment:
yes, makes sense, I did not want to change the walrus operator, but this look more readable.
--
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