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
GitHub actions for testing #49
Conversation
Looks that way! I can see this PR on the Actions page! |
It dies with the same weird error:
But it goes quite fast, just 14 minutes. Looks like maybe the Spark import does not work out. |
Thanks, promising! Shall I review or wait until you got to the bottom of the compile issue? |
Thanks! Yeah, don't review it yet. |
Now it's actually running our tests! Most of them even pass. I've seen a failure that I cannot reproduce locally though:
|
|
267cdb3
to
d9084d9
Compare
d9084d9
to
9661546
Compare
05b65a7
to
7d5e8ac
Compare
Hallelujah! 🎉 I'm sure we can improve them but it's a start!
But I think we can merge it for now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't say I thoroughly understood all details, but looks good in general! Cool that we have a working CI now! :)
@@ -1,13 +1,17 @@ | |||
#!/bin/bash -xue | |||
# Install PyTorch Geometric. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pytorch is our only dependency? Maybe worth renaming this file? I mean, someone might innocently add something at the end and get surprised by it not working, due to the if at the start. Or maybe reverse the if. and put the torch related pip installs inside? Btw, why the if? Doesn't pip handle this anyways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pytorch is our only dependency? Maybe worth renaming this file? I mean, someone might innocently add something at the end and get surprised by it not working, due to the if at the start. Or maybe reverse the if. and put the torch related pip installs inside? Btw, why the if? Doesn't pip handle this anyways?
Without the if
it takes 40 seconds to do nothing. We could probably use Pipenv or some magic to handle this better, but I'm just happy it works at all. 😅
I've reversed the if
, thanks for the suggestion!
For #8. It's hard to test this locally. I'm using https://github.com/nektos/act but I'm getting weird errors and caching doesn't work, so each attempt takes ages. Will this PR trigger a run, I wonder? If not, I may merge this and try to see if I can trigger it that way.