OpenWaterAnalytics / epanet-python

python wrapper for epanet library
76 stars 40 forks source link

Pivot epanet-python packaging to binary distribution #75

Open michaeltryby opened 3 years ago

michaeltryby commented 3 years ago

I had a chance to review the Issues posted to this repository and it seems like build and install problems have been coming up frequently.

Is there any interest in pivoting from a source distribution to a binary distribution for an EPANET wrapper package?

Over at OWA/swmm-python we recently completed work on a binary distribution for SWMM.

https://github.com/OpenWaterAnalytics/swmm-python/tree/dev/swmm-toolkit

https://pypi.org/project/swmm-toolkit/

It would be straight forward to use swmm-toolkit as a template and create an epanet-toolkit wheel.

Such a wheel would resolve many of the build and install issues Python developers are experiencing here on the EPANET side.

samhatchett commented 3 years ago

this is a great idea. I've been meaning to learn more about GH actions, but I've been spending most of my time in the Atlassian world unfortunately.

looking over at swmm-python, it looks like the actions are producing lots of wheels and saving them - I assume there would be a way to auto-publish to pypi as well. What I've done here is to manually build on Mac and upload the sdist alongside the Mac wheel.

Is there anything beyond the actions/scripts that needs to be figured out, and would that be the first step here?

michaeltryby commented 3 years ago

Fantastic! There were a number of obstacles we overcame to make the swmm-toolkit wheel build robust on Windows, Linux, and Mac.

Each platform finds shared libraries at runtime using a different strategy. For the wheel to work the SWIG generated wrapper library needs to find the epanet library at runtime at the wheel install location. Thankfully CMake has tools for setting "rpaths" that make this all straightforward and manageable.

Scikit-build will work a lot better if there is an install target for the epanet library. It uses install targets to stage files then hands off to python tools to create the wheel package.

So the epanet library build system will need to be updated to create an install target and set the rpath for Linux and Mac. It makes sense to propagate these changes up into the epanet repository. Fortunately, this can be accomplished so that developers not interested in building Python wheels will be uneffected.

As a first step I can put together a PR. I actually have a lot of the work done already. Sound good?

There probably is a way to upload the wheels direct to pypi but I haven't investigated it ...

samhatchett commented 3 years ago

sure, I think a PR would be great if you already have some work ready to look over. Anybody else have opinions?

on the epanet install target, sounds like that overlaps with this issue -> https://github.com/OpenWaterAnalytics/EPANET/issues/638 - might be worth pinging that thread?

michaeltryby commented 3 years ago

Great! I will pull together a PR that way we can have something concrete to look at.

michaeltryby commented 3 years ago

Taking a moment to reflect … it’s kinda poor etiquette to drop a huge PR on your project. Would you all prefer to reuse the work over in swmm-toolkit as a template for epanet-toolkit? Or would you prefer to automate the wheel build for your existing owa-epanet package? Happy to do either. Let me know how you would like to proceed.

samhatchett commented 3 years ago

how huge are we talking? sounds like you might have alternate swig interface files? just trying to get a handle on the scope, since this thread's starting point was related to packaging and distribution. Can you elaborate on the phrase "reuse the work" - which work in particular?

michaeltryby commented 3 years ago

Just seeing this now … The CI workflow automation is really the tip of the iceberg. Setting up the build and troubleshooting the wheels took most of the development effort. The entire swmm-toolkit project and build setup can be reused like a project template to produce an epanet-toolkit wheel. My vote is for reuse. So the scope would involve reworking the project layout, CMakeLists, and setup.py. The swig wrapper interface definition file can be reused. That doesn’t affect the wheel build setup. Thoughts?

michaeltryby commented 3 years ago

@samhatchett created a PR with a basic Actions workflow automating the current wheel build on Mac OS. You need to merge it to activate Actions.

samhatchett commented 3 years ago

cool, so I merged that PR - but the file seems to not be fully correct. Can you troubleshoot given your experience with Actions?

Also, maybe you can suggest a new project layout in a different Issue, so that can be more fully explored? thanks

michaeltryby commented 3 years ago

Had some time available today to work on this ... discovered several issues with the wheels #78, #79, #80, #81

samhatchett commented 3 years ago

the first three seem to be basically the same issue, right? and the last one is a windows-ey thing... all having to do with setting up the build and troubleshooting the wheels?

And I imagine that since "The entire swmm-toolkit project and build setup can be reused", if we do so then these issues go away?

There's no need to piecemeal it. If your experience with wrapping SWMM has led you to certain insights, please describe them and let's discuss how we get there. You could propose a new project layout, and talk about how that would help things. Maybe there are threads on the swmm project that help one to understand build/link settings. I also have heard no objections to opening a huge PR.

michaeltryby commented 3 years ago

I'm going to move ahead by preparing the PR reusing swmm-toolkit as a template for a sister epanet-toolkit package. You seem open to that approach so it will be worth finishing that up. Might take me awhile since I am juggling several other things at the moment.

You know your owa-epanet package was pretty useful in that it established scikit-build as a viable way to build python extensions. I didn't have enough CMake XP to get scikit-build to work the first time I tried. The swmm-toolkit package took your work and built on it. So its great to be able to bring this work back to epanet more fully realized.