Skip to content

Allow non offset based timezones #1078

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

Closed
Fokko opened this issue Mar 12, 2025 · 1 comment · Fixed by #1086
Closed

Allow non offset based timezones #1078

Fokko opened this issue Mar 12, 2025 · 1 comment · Fixed by #1086
Labels
bug Something isn't working

Comments

@Fokko
Copy link
Contributor

Fokko commented Mar 12, 2025

Apache Iceberg Rust version

None

Describe the bug

I was trying to make PyIceberg rely solely on Iceberg-rust for the partition transforms, but I ran into the following:

    def _transform(array: "ArrayLike") -> "ArrayLike":
        if isinstance(array, pa.Array):
            return transform_func(array, *args)
        elif isinstance(array, pa.ChunkedArray):
            result_chunks = []
            for arr in array.iterchunks():
>               result_chunks.append(transform_func(arr, *args))
E               ValueError: Unexpected => Parser error: Invalid timezone "UTC": only offset based timezones supported without chrono-tz feature

We initialize the datetime as:

datetime(2023, 12, 31, 0, 0, 0, tzinfo=timezone.utc)

To Reproduce

In test_transform.py:

def test_year_transform_datetime():
    arr = pa.array([
        datetime(1970, 1, 1, 0, 0, 0, tzinfo=timezone.utc),
        datetime(2000, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
    ])
    result = transform.year(arr)
    expected = pa.array([0, 30], type=pa.int32())
    assert result == expected

Expected behavior

Able to just parse these. I'm not sure if we need to fix this in Arrow, or if we can import chrono-tz package.

Willingness to contribute

I cannot contribute a fix for this bug at this time

@liurenjie1024
Copy link
Contributor

Seems this is cause by arrow, see https://github.com/apache/arrow-rs/blob/d5339f31a60a4bd8a4256e7120fe32603249d88e/Cargo.toml#L98C51-L98C56 .

The temporal transforms are using arrow to do the transform:

let array = date_part(&input, DatePart::Year)

Fokko added a commit that referenced this issue Mar 18, 2025
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->

- Closes #1078.

## What changes are included in this PR?

<!--
Provide a summary of the modifications in this PR. List the main changes
such as new features, bug fixes, refactoring, or any other updates.
-->
Enable `chrono-tz` feature in the `arrow` dependency
Thanks to this reference, delta-io/delta-rs#2295

## Are these changes tested?

<!--
Specify what test covers (unit test, integration test, etc.).

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
Yes. Unit test coverage using datetime with timezone

Co-authored-by: Fokko Driesprong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants