Skip to content

GH-31318: [Python] Add fixed-offset timezones to Hypothesis test strategy#49844

Open
alex-anast wants to merge 3 commits intoapache:mainfrom
alex-anast:alex-anast/gh-31318-python-tz-fixed-offset-strategy
Open

GH-31318: [Python] Add fixed-offset timezones to Hypothesis test strategy#49844
alex-anast wants to merge 3 commits intoapache:mainfrom
alex-anast:alex-anast/gh-31318-python-tz-fixed-offset-strategy

Conversation

@alex-anast
Copy link
Copy Markdown

@alex-anast alex-anast commented Apr 22, 2026

Rationale for this change

The Hypothesis test strategy for timezones (pyarrow.tests.strategies.timezones) only generates named IANA timezones (via pytz and zoneinfo). It does not generate fixed-offset timezones like +05:30 or -03:00. As noted in #31318, this is not supported out of the box by Hypothesis, so a custom strategy is needed. Fixed-offset timezones are already used in manual tests and handled correctly by Arrow, but are never exercised via property-based testing.

What changes are included in this PR?

Adds a fixed_offset_timezones strategy to python/pyarrow/tests/strategies.py that generates datetime.timezone objects with offsets ranging from UTC-12:00 to UTC+14:00, with minute components of 0, 30, or 45 (covering all real-world UTC offsets). This strategy is included in the existing timezones strategy across all dependency configurations.

Are these changes tested?

This change improves test infrastructure itself -- it extends the timezones strategy used by existing Hypothesis-based tests. The generated fixed-offset timezones were verified to work correctly with pa.timestamp().

The test command used was:

export MAMBA_ROOT_PREFIX=$PWD/.micromamba && eval "$(/tmp/bin/micromamba shell hook -s bash)" && micromamba activate pyarrow-dev && export  LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH && python -m pytest pyarrow/tests/test_scalars.py pyarrow/tests/test_array.py -v -k test_dunders 

Are there any user-facing changes?

No. This only affects the internal test suite.

AI-generated code disclosure

This PR was developed with assistance from an AI coding tool (Claude, Anthropic). All changes have been reviewed, understood, and verified.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #31318 has been automatically assigned in GitHub to PR creator.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #31318 has been automatically assigned in GitHub to PR creator.

@raulcd
Copy link
Copy Markdown
Member

raulcd commented Apr 23, 2026

@github-actions crossbow submit test-conda-python-3.11-hypothesis

@github-actions
Copy link
Copy Markdown

Revision: 6c9a20e

Submitted crossbow builds: ursacomputing/crossbow @ actions-16091e66e1

Task Status
test-conda-python-3.11-hypothesis GitHub Actions

@AlenkaF
Copy link
Copy Markdown
Member

AlenkaF commented Apr 23, 2026

The extended hypothesis failure is currently unrelated, opened an issue for it here: #49846

Copy link
Copy Markdown
Member

@AlenkaF AlenkaF left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
The only comment I have for now is that the current strategy can produce UTC out of the valid UTC range which is UTC-12 and UTC+14, for example UTC-12:45. Datetime will accept it but as this is invalid IANA timezone it should be improved, it also might produce errors in other parts.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #31318 has been automatically assigned in GitHub to PR creator.

2 similar comments
@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #31318 has been automatically assigned in GitHub to PR creator.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #31318 has been automatically assigned in GitHub to PR creator.

@alex-anast
Copy link
Copy Markdown
Author

alex-anast commented Apr 23, 2026

@AlenkaF thanks for the review!
You're right. I've pushed a follow-up that enforces both bounds, making sure that no offset fails outside the valid UTC-12 to UTC+14 range. Can you please re-check?

edit: on hindsight, a lower bound filter is unnecessary since the lower bound can never be violated -- harmless, clear and defensive, but dead code.

@AlenkaF
Copy link
Copy Markdown
Member

AlenkaF commented Apr 23, 2026

Thanks for the update! There is a PR up to fix the hypothesis build: #49847. After that is merged you can rebase and we can run the build here, to double check the logic.

@raulcd
Copy link
Copy Markdown
Member

raulcd commented Apr 24, 2026

@alex-anast the PR has been merged on main, if you could rebase I'll re-trigger the hypothesis tests on CI

@alex-anast alex-anast force-pushed the alex-anast/gh-31318-python-tz-fixed-offset-strategy branch from 564317e to 820665d Compare April 24, 2026 10:44
@alex-anast
Copy link
Copy Markdown
Author

@github-actions crossbow submit test-conda-python-3.11-hypothesis

@github-actions
Copy link
Copy Markdown

Only contributors can submit requests to this bot. Please ask someone from the community for help with getting the first commit in.
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/24885507497

@alex-anast
Copy link
Copy Markdown
Author

Only contributors can submit requests to this bot. Please ask someone from the community for help with getting the first commit in.
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/24885507497

expected 😓

@raulcd can you please re-trigger the test when you get the chance?

@raulcd
Copy link
Copy Markdown
Member

raulcd commented Apr 24, 2026

@github-actions crossbow submit test-conda-python-3.11-hypothesis

@github-actions
Copy link
Copy Markdown

Revision: 820665d

Submitted crossbow builds: ursacomputing/crossbow @ actions-096afcf5c0

Task Status
test-conda-python-3.11-hypothesis GitHub Actions

@AlenkaF
Copy link
Copy Markdown
Member

AlenkaF commented Apr 24, 2026

I think the failure is connected to the changes in this PR:

TypeError: tzinfo argument must be None or of a tzinfo subclass, not type 'datetime.timedelta'

see: https://github.com/ursacomputing/crossbow/actions/runs/24890383634/job/72880860677#step:6:3978

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Python] Update timezones strategy to include fixed offsets

3 participants