Skip to content
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

GH-44369: [CI][Python] Remove ds requirement from test collection on test_dataset.py #44370

Merged
merged 2 commits into from
Oct 11, 2024

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Oct 10, 2024

Rationale for this change

If pyarrow has been built without dataset test collection errors due to pytest.mark.parametrize requiring dataset.

What changes are included in this PR?

Move dataset requirement outside of test collection by removing it from the parametrization.

Are these changes tested?

Via archery

Are there any user-facing changes?

No

Copy link

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

@raulcd
Copy link
Member Author

raulcd commented Oct 10, 2024

@github-actions crossbow submit example-python-minimal-build-*

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Oct 10, 2024
Copy link

Revision: d682c4f

Submitted crossbow builds: ursacomputing/crossbow @ actions-b410c377ee

Task Status
example-python-minimal-build-fedora-conda GitHub Actions
example-python-minimal-build-ubuntu-venv GitHub Actions

@raulcd raulcd marked this pull request as ready for review October 10, 2024 14:43
@raulcd raulcd requested a review from amoeba October 10, 2024 14:43
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Oct 10, 2024
Copy link
Member

@amoeba amoeba 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 fixing this!

@kou kou merged commit e0d1f51 into apache:main Oct 11, 2024
16 checks passed
@kou kou removed the awaiting merge Awaiting merge label Oct 11, 2024
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit e0d1f51.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

raulcd added a commit that referenced this pull request Oct 11, 2024
…test_dataset.py (#44370)

### Rationale for this change

If pyarrow has been built without dataset test collection errors due to pytest.mark.parametrize requiring dataset.

### What changes are included in this PR?

Move dataset requirement outside of test collection by removing it from the parametrization.

### Are these changes tested?

Via archery

### Are there any user-facing changes?

No
* GitHub Issue: #44369

Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
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.

3 participants