-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Conversation
@github-actions crossbow submit test-conda-python-3.11 |
Revision: 49f12ed Submitted crossbow builds: ursacomputing/crossbow @ actions-0f40117f8a
|
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. |
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 |
@github-actions crossbow submit test-conda-python-3.11 |
Revision: 2bdb735 Submitted crossbow builds: ursacomputing/crossbow @ actions-257d34b315
|
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 |
I was expecting the last one to fail 🤔 |
Hmm, yeah, that's also my expectation .. |
I assume it doesn't see the setup.cfg, because we run the installed tests, not in-place (using |
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? |
This is just a testing PR related to: #38238