-
Notifications
You must be signed in to change notification settings - Fork 22
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
Python tests failing in CI #186
Comments
FWIW, there are also a lot of other warnings showing up in the python build step. I found 81 hits when searching for Those warnings may or may not be related to this particular failure, but I think they should definitely be looked at and cleaned up at some point. One of the general rules that was agreed for our NCEPLIBS packages was that everything should compile without warnings. |
BTW, #135 is still open and may also be related. |
Digging further, I'm also seeing:
and:
But again, I'm not really sure what I'm looking for here. @aerorahul @jswhit2 @edwardhartnett could I ask one of you to please take a look at this? As noted in #185, this was all working fine back in November when I released v11.6.0, and which is the last time anybody tried to do a merge to the |
Yeah, this is a little out of my Python knowledge. The offending code is in the Python CMake here: NCEPLIBS-bufr/python/CMakeLists.txt Line 21 in 3f2e10b
My guess is the Python version was updated in the Github runner. |
It looks like we now have another PR #189 (in addition to PR #185) that we can't merge into the |
Since nobody else has been able/willing to step up and help, I went snooping on the Internet for some possible solutions. (I think?) I made some progress, but still not there yet. Please see #190 and my branch This is way above my level of knowledge w.r.t. python, so I'm not sure I'm even on the right track here, but then again I also don't think I'm the person who should be stuck with trying to fix this. The latest error in the build is now:
but I have no idea what "requirement" I should list here, or what that really even means. For now at least, I've also taken out the previous --record option, because I couldn't find a related option under PLEASE, could somebody more knowledgeable about python installs help resolve this issue? This is preventing anything else from getting merged into the |
sorry for not contributing to this, but I don't know what the right way to handle python package installations in cmake is. |
There is a simple fix. There are deeper issues about how the Python bindings are handled, but if Python can find the bindings the tests will work. |
This is actually a suprisingly difficult problem on python projects, and I have run into it before. The usual answer I have seen is to install the package with a command like:
|
I'll take a look at it today. |
I was able to get the tests running by manually setting But I think there's a better way of doing it. |
Hey @aerorahul, just wanted to touch base. Have you been able to make any progress on this? |
I have failed to reproduce this on my mac. Is this something to do in the github action? |
Maybe. But either way, it's something that I was hoping I could get some help with from someone more knowledgeable than me. |
Whatever the problem is, it's definitely in the GitHub runner. So even if you can't reproduce it on your local machine, you can still see it if you try pushing it to GitHub. An example is #191 that @kgerheiser tried a couple of weeks ago. It apparently worked when he tested it manually, but the GitHub runner checks for that PR still failed. |
Just checking in - any progress on this? |
All the Python tests fail in the CI.
Also this a bit further up:
Not sure why the change, but it seems that ncepbufr is not in the PYTHONPATH.
The text was updated successfully, but these errors were encountered: