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

MINOR: WIP [Python][CI] Enable -W error on all Python tests #38259

Closed
wants to merge 2 commits into from

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Oct 13, 2023

This is just a testing PR related to: #38238

@raulcd
Copy link
Member Author

raulcd commented Oct 13, 2023

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

@github-actions
Copy link

Revision: 49f12ed

Submitted crossbow builds: ursacomputing/crossbow @ actions-0f40117f8a

Task Status
test-conda-python-3.11 Github Actions

@jorisvandenbossche
Copy link
Member

I thought about that yesterday as well, but this won't work without adding some more ignore filters. For example on the nightly builds with numpy/pandas we still have some deprecation warnings.

The annoying part with this way is that we can't tweak it per build like with the env var, but of course we can then just add enough filters to cover all builds, and don't care that they are too broad for some other builds. And in any case, this solves the whole issue of not being able to pass it through an env var if the filter has spaces as we have in #38238. And another benefit is that this then also is used when running the tests locally, making it more consistent with CI.

@raulcd
Copy link
Member Author

raulcd commented Oct 13, 2023

Yes, I was pretty sure we would have to add more tweaking for other jobs but to be honest at the moment I am not entirely sure how to fix it the other way, unless we want to add a much broader ignore (without spaces).

There also some builds where we might just want to add -W ignore and we basically don't get any error like the nightly builds with numpy/pandas. So probably is worth exploring how much effort this is going to take.

@raulcd
Copy link
Member Author

raulcd commented Oct 13, 2023

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

@github-actions
Copy link

Revision: 2bdb735

Submitted crossbow builds: ursacomputing/crossbow @ actions-257d34b315

Task Status
test-conda-python-3.11 Github Actions

@jorisvandenbossche
Copy link
Member

There also some builds where we might just want to add -W ignore

Ah, indeed, we could override those settings on certain builds like that (although we don't want to ignore them, but always show them, so -W always?)

@raulcd
Copy link
Member Author

raulcd commented Oct 13, 2023

I was expecting the last one to fail 🤔
My expectation was the CLI -W was taking precedence but if no input on CLI the setup.cfg config should have kicked in, so I was expecting if we don't add the ignore the build to fail 🤔

@jorisvandenbossche
Copy link
Member

Hmm, yeah, that's also my expectation ..

@jorisvandenbossche
Copy link
Member

I assume it doesn't see the setup.cfg, because we run the installed tests, not in-place (using --pyargs pyarrow)

@jorisvandenbossche
Copy link
Member

Short term, while we figure this out, maybe we can just remove the "-W error" for now in #38238 so that the nightly CI is green again. And then we can do a follow-up PR like this one to figure it out in a more robust way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants