-
Notifications
You must be signed in to change notification settings - Fork 140
Pypi packaging #73
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
Pypi packaging #73
Conversation
|
@pablospe Currently it will build on push to master, which I think is reasonable. I guess for other CI stuff we want to run on PRs, especially the test suite. Not sure how extensive we want to test, the current builds are for a wide range of setups, but that takes a very long time to run. Perhaps limiting the testing to just some ubuntu/python combo should be enough. |
|
In order to have a Github Action, you will need to set-up one first by clicking here: Then copy the files content of this PR to the new one, and close this PR to avoid confusion. |
|
@vlarsson Is there any news about this PR? |
|
I will have a look at this next week after the 3DV camera ready deadline. So this should upload to pypi when we make new releases? or I am not sure I am understanding the build-pypi.yaml. |
|
you will probably need to create a new github action, see my previous message: |
|
@Parskatt You might need to create a Github Action first, by clicking here: Then copy the files content of this PR to the new one. The github actions from |
|
Sorry, I did not mean to cause this review. Was just gonna fix stuff over at my fork. Perhaps we can disable the autoreview? I'll get back to you later. |
|
@pablospe Ok, but the action won't run without the changes to rest in the PR, if I push the action first it will start running on pushes to master. Wouldn't that be kind of annoying? Maybe I'm misinterpreting you. Also some general discussion:
|
I was suggesting to create another PR where the actions are active, that's all, as now the If you see here: And one also wants to run it on master because it takes long time to run, and because you don't want to push artifacts to pypi from untested and non-approved branch.
Not sure what you mean. The action will run in the current branch on the PR, later you will need to filter and say the publishing of the artifact should run only in master, so when you publish a package you know it is from master, but this has to be done explicitly (it is not the default). Probably we are talking about something different?
This is fine, just wondering how it works with the python bindings? Doesn't it needs a .so? (at least in linux)
You can also check the benchmark result when the action is running: |
|
|
@pablospe The issue is that we're building wheels, and the binaries don't end up in any convienent place as far as I can tell. In the basic CI that you already made, its easy to set the install_dir and simply run, but when building wheels I don't know where these targets end up. Regarding testing the wheels, yes, they are already set up and running on my fork. I guess I can set up a new PR with your approach, so that the action runs there |
|
Now it is running, see the changes (you need to revert these before merging to PR, now it is for testing until you get it running): So, it wasn't triggering because in your PR/branch because you added: For testing you can comment it out, then when things are ready you can put it again so it only runs on master for the following PRs. |
|
should we close this PR? And move to the new one you created? |


Resolves #72
Added build for multiple platforms. Also some fixes for platform specific issues.
Mac complained about "aligned-allocation" so compiling without it.
Windows broke because the MSVC flag was missing in the pybind cmakelists.
Currently no integration with the test suite, should add.
Also wheels seem to be sometimes broken, so we need to test these on some different systems before pushing pypi.