-
Notifications
You must be signed in to change notification settings - Fork 9
Add packaging of MeshPy to testing #335
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
base: main
Are you sure you want to change the base?
Conversation
eb8fa4e
to
1f97974
Compare
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.
@isteinbrecher thanks a lot for tackling this in the first place!
It's really nice that the wheel builds and works as intended. I am not 100% happy with the workflow though. I think we should not bring the wheel building process into our testing procedure.
Not too long ago I invested some time into the wheel building process of rapidyaml (https://github.com/biojppm/rapidyaml).
If we want to publish the wheels to pypi (hopefully in the near future) we need to build the wheels for a lot of systems (i.e., ubuntu, windows, macos and for all python versions => at least 15 different wheels)
In my opinion we should simply make a new workflow "build (and publish) wheels" and build all the wheels via a matrix there. For now we could upload the resulting wheels as an artifact.
In testing we should then download the wheel from the other completed job and install it. Additionally, I think it would be nice to keep the non editable install testing (to be on the safe side) and add the installation via a wheel.
It should be pretty straight forward to update the current process to my proposed one. Let me know what you think:)
Thanks for your thoughts @davidrudlstorfer , my initial thoughts behind this PR were the following: Building of the wheel did not work, since the Cython source files were not copied, so we needed the As you said, once we go to pypi, we have to build for many different versions and OS systems. The question is, if we want to do that for every pipeline run or just for special cases, e.g., merging to I think that there is not much difference between an installation from a wheel and the editable installation - so I thought testing just one is fine. But after second thought, you are right, let's keep both, one never knows. We can try to figure out a way how we want to test the installation / building in the near future (publishing to pypi will be another topic) and then apply this within this PR. @davidrudlstorfer I have no strong feelings here and sine you are the MeshPy testing expert I am very curious on your opinion. Would something like this make sense:
|
@isteinbrecher I think this sounds like a solid plan. As you've mentioned the pipelines of MeshPy are that fast (and they are running in parallel) that we can add more. In the last few months we've seen that everything works perfectly and we do not have any problems with all of the versions. Therefore, I think we can simply also just build all wheels in the PR pipelines (to ensure that it works) and only push them in main. Regarding the setup for the pipelines I think that we can also do everything in parallel. The code checks and the testing occures in parallel right now. Also the wheels could be built in parallel to testing. Only the testing in combination with wheels depend on the prior built wheels. These could automatically start once the wheel is available. |
After discussing last week with @clemens-fricke how this is done in splinepy, I propose to replace the current "non-editable" install test by tests where we first build the MeshPy wheel and then install the wheel. By doing so, we should be able to somewhat guarantee that changes to the code don't affect the packaging behaviour. @clemens-fricke do you agree?