Skip to content

Fix creation of Bucket Transforms with pydantic>=2.11.0 #1881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 4, 2025

Conversation

b-rick
Copy link
Contributor

@b-rick b-rick commented Apr 3, 2025

Rationale For This Change

When using pydantic>=2.11.0, we get an error when creating bucket transforms. In this version, it's illegal to access self before calling super. To fix this, we just need to ensure we call super().__init__ before setting field properties on self.


Before

Using pydantic==2.11.0 the test test_transforms.py::test_bucket_hash_values fails

tests/test_transforms.py:None (tests/test_transforms.py)
test_transforms.py:179: in <module>
    (BucketTransform(2).transform(IntegerType()), 0, 0),
../pyiceberg/transforms.py:237: in __init__
    self._num_buckets = num_buckets
../../../miniforge3/envs/pyiceberg/lib/python3.12/site-packages/pydantic/main.py:991: in __setattr__
    setattr_handler(self, name, value)  # call here to not memo on possibly unknown fields
../../../miniforge3/envs/pyiceberg/lib/python3.12/site-packages/pydantic/main.py:105: in <lambda>
    'private': lambda model, name, val: model.__pydantic_private__.__setitem__(name, val),  # pyright: ignore[reportOptionalMemberAccess]
E   AttributeError: 'NoneType' object has no attribute '__setitem__'. Did you mean: '__setattr__'?

After

Using pydantic==2.11.0 the test test_transforms.py::test_bucket_hash_values succeeds.


✅ Are these changes tested?

Yes

  • No test was added to reproduce the bug because it is already reproducible with the existing test

When using pydantic>=2.11.0, we get an error when creating bucket
transforms. In this version, it's illegal to access self before calling
super. To fix this, we just need to ensure we call `super().__init__`
before setting field properties on `self`.
@Fokko Fokko added this to the PyIceberg 0.9.1 milestone Apr 4, 2025
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Great catch @b-rick Thanks for fixing this 🚀

@Fokko Fokko merged commit a62799e into apache:main Apr 4, 2025
7 checks passed
Fokko pushed a commit that referenced this pull request Apr 17, 2025
## Rationale For This Change

When using pydantic>=2.11.0, we get an error when creating bucket
transforms. In this version, it's illegal to access self before calling
super. To fix this, we just need to ensure we call `super().__init__`
before setting field properties on `self`.

---

## Before

Using `pydantic==2.11.0` the test
`test_transforms.py::test_bucket_hash_values` fails

```python
tests/test_transforms.py:None (tests/test_transforms.py)
test_transforms.py:179: in <module>
    (BucketTransform(2).transform(IntegerType()), 0, 0),
../pyiceberg/transforms.py:237: in __init__
    self._num_buckets = num_buckets
../../../miniforge3/envs/pyiceberg/lib/python3.12/site-packages/pydantic/main.py:991: in __setattr__
    setattr_handler(self, name, value)  # call here to not memo on possibly unknown fields
../../../miniforge3/envs/pyiceberg/lib/python3.12/site-packages/pydantic/main.py:105: in <lambda>
    'private': lambda model, name, val: model.__pydantic_private__.__setitem__(name, val),  # pyright: ignore[reportOptionalMemberAccess]
E   AttributeError: 'NoneType' object has no attribute '__setitem__'. Did you mean: '__setattr__'?
```

---

## After

Using `pydantic==2.11.0` the test
`test_transforms.py::test_bucket_hash_values` succeeds.

---

✅ Are these changes tested?

Yes

- No test was added to reproduce the bug because it is already
reproducible with the existing test

Co-authored-by: Zac Sanchez <[email protected]>
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