search

OpenWaterAnalytics / epanet-python

python wrapper for epanet library
76 stars 40 forks source link

Made changes to enable compilation on Windows #56

Closed sm-rana closed 4 years ago

sm-rana commented 4 years ago

Added a .bat file to invoke compilation of the python wrapper for EPANET 2.2 from within a Windows based system. Small changes to the CmakeList.txt were made.

dly9000 commented 4 years ago

I was able to build the toolkit.py, _toolkit.pyd, and epanet.dll for windows using the .bat and cmake file in this pull request. It was built it in a conda environment and I do not think it ended up built as a package, i.e. 

    from epanet import toolkit

does not work.

The files need to be in the project woring directory to use it like the epamodule.py file.
Results wise, it seems to be working correctly though.

sm-rana commented 4 years ago

@dly9000 You are correct -- the output files are currently copied outside the project folder (which is how it was originally in the CmakeList.txt). @samhatchett is there a reason for the files to be copied outside the project folder?

dly9000 commented 4 years ago

@sm-rana I am completely ok with it working that way. I am just trying to be sure I have built it and am using it correctly. Thank you for developing the .bat file for windows!

@samhatchett If the test_owa_epanet.py file runs wiythout any output, does that mean the tests have passed? Are there additional tests that can be written to increase test coverage?

samhatchett commented 4 years ago

@dly9000 re/ the test output... @ernestoar recently contributed those and so could perhaps answer.

Regarding the auto-copying of compiled output to a location external to the project folder, I think this is fairly common in python packaging, to ensure that the libraries are accessible to the loader. Otherwise LD_LIBRARY_PATH would have to be altered. I'm likely missing something - using the SWIG docs for reference: http://www.swig.org/Doc4.0/SWIGDocumentation.html#Scripting_nn11

Anyone have suggestions on how else to handle?

ernestoar commented 4 years ago

@sm-rana @samhatchett The answer is yes to both. Silent output means that tests have passed. Additional tests can be written, since not all the toolkit functions are covered at the moment. I'll look into it.

sm-rana commented 4 years ago

@dly9000 thanks a lot for testing the win_build. @samhatchett @ernestoar I have finally realized why the library paths are the way it was. It had to do with the folder structure created by scikit-build. I fixed the .bat file, so the libraries are no longer copied outside the project folder.

Also with the changes made in CmakeList.txt, I am now able to run python setup.py develop or python setup.py sdist bdist_wheel to successfully build on Windows, which should be the preferred way to build it instead of using the .bat file.