OpenWaterAnalytics / epanet-python

python wrapper for epanet library
76 stars 40 forks source link

SWIG generated Toolkit API wrapper? #5

Closed michaeltryby closed 6 years ago

michaeltryby commented 6 years ago

Hey @mauriziocingi I have worked with ctypes, ctypesgen, and now SWIG for integrating our EPANET outputapi library with python. You can see the latest SWIG version here.

I am really happy with how it came out. Using SWIG it is easy to handle memory management, error handling, and best of all the library becomes a fully fledged Python extension. I was thinking about SWIG wrapping the Toolkit API. Is that something that could live in this repo along with your work?

I also created several Python packages related to regression testing that need a home. We are contemplating a bit of a reorganization over in the EPANET repo in Issue #123. Would you be amenable to keeping multiple EPANET related python packages here in your repo? Give it some thought and let us know what your preferences are. Thanks!

samhatchett commented 6 years ago

I will agree with this suggestion - seems that starting with a clean, auto-generated python API wrapper is a good foundation for building more abstractions. This also intersects with the near-term needs of the GUI work (which uses python and QT) - SWIG-wrapping will mean that the epanet library becomes scriptable from within the GUI.

Another benefit of auto-generating the wrapper is that it's fairly unambiguous; nobody's personal preferences get involved until we get to slightly higher-level abstractions, but everyone can share and benefit from the foundational SWIG layer.

It makes enough sense to me that the only real question is where the SWIG wrapper should live. This repo is a good candidate. We could also set up another one - but that might get confusing to have two "python wrapper" repositories in the same organization.

What do you think, @mauriziocingi ?

mauriziocingi commented 6 years ago

I think there are two different point of view.

I also think that new Gui cannot use C data structures but create its own py classes and it has to be able to export/files INP files, run epanet (as epanet2d or via toolkit) and then read BIN output files. A plus could be a spatial database (sqlite+ spatialite as light solution, postgresql+ postgis the heavy one) as data repository and link to/from GIS. (At present I'm using epamodule.py as bridge between epanet and Grass Gis/ Qgis.

This is my (personal) point of view but I should confess that I've not enough time and I cannot guarantee adequate maintanance/devolepement to epamodule.

samhatchett commented 6 years ago

I think the real need that would be addressed by adding the SWIG layer is in being able to access the wrapped C API functions more directly. SWIG may not provide a "pythonic" interaction model, but at the very least the toolkit functions would become python-scriptable. Note especially that this includes the new set of functions for adding/removing network elements, and if we standardize on SWIG then future enhancements to the C api will automatically become available to python users.

The current qgis/python GUI does have its own data model - so it parses an input file, populates the python data model, the user can make changes (to the python data model). Then to run a simulation, the GUI has to serialize to an inp file, and then load that file using epanet's C file-loading api function, run, and then read the binary results file. Whew.

The preferred paradigm would be to let the epanet C library load the file, use the SWIG-wrapped API to inspect the C data model, to populate the UI. As the user makes changes to the network, the actual C api calls would be made under the hood via the SWIG wrapper. If the user runs a simulation, then epanet's own calls are used to step through the sim, rather than juggling temporary inp files.

The real point of all this is to let epanet manage the data model, and to access that model through the given api calls - that way the GUI doesn't have to manage its own duplicative data model.

As to the "pythonic" interaction model, that can co-exist happily with a SWIG-wrapped api, just as an abstraction build on top - rather than itself using C calls.

@mauriziocingi are you agreeable to opening up this repo to other committers (like @michaeltryby and myself) to more fully explore these ideas? If not, it is perfectly fine and we can set up another repo for the SWIG wrapper.

michaeltryby commented 6 years ago

@samhatchett You make some good points. If @mauriziocingi is agreeable, I propose we preserve his epamodule code here by moving it in to its own subfolder so as to not adversely affect his work. The python packages could then all live side by side and be contained in their own folders within this repo.

mauriziocingi commented 6 years ago

Sure, I agree. Maybe I have something to do for give you r+w rights on this repo? (I suppose @samhatchett https://github.com/samhatchett already has admin previleges on the repo) In the case just let me know.

2017-12-06 15:35 GMT+01:00 Michael Tryby notifications@github.com:

@samhatchett https://github.com/samhatchett You make some good points. If @mauriziocingi https://github.com/mauriziocingi is agreeable, I propose we preserve his epamodule code here by moving it in to its own subfolder so as to not adversely affect his work. The python packages could then all live side by side and be contained in their own folders within this repo.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenWaterAnalytics/epanet-python/issues/5#issuecomment-349657540, or mute the thread https://github.com/notifications/unsubscribe-auth/ALVwWRmDNGXRcSzYTvwn89qUbxigVV7lks5s9qZMgaJpZM4QLf_H .

samhatchett commented 6 years ago

thanks @Mariosmsk - I've added myself and @michaeltryby to the epanet-python-authors group, which will give us all write privileges. 👍