{"action":"created","issue":{"url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/issues/54","labels_url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/issues/54/labels{/name}","comments_url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/issues/54/comments","events_url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/issues/54/events","html_url":"https://github.com/OpenWaterAnalytics/EPANET/issues/54","id":121792881,"number":54,"title":"evaluate / tweak Python wrapper for binary output api ","user":{"login":"samhatchett","id":2386549,"avatar_url":"https://avatars.githubusercontent.com/u/2386549?v=3","gravatar_id":"","url":"https://api.github.com/users/samhatchett","html_url":"https://github.com/samhatchett","followers_url":"https://api.github.com/users/samhatchett/followers","following_url":"https://api.github.com/users/samhatchett/following{/other_user}","gists_url":"https://api.github.com/users/samhatchett/gists{/gist_id}","starred_url":"https://api.github.com/users/samhatchett/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/samhatchett/subscriptions","organizations_url":"https://api.github.com/users/samhatchett/orgs","repos_url":"https://api.github.com/users/samhatchett/repos","events_url":"https://api.github.com/users/samhatchett/events{/privacy}","received_events_url":"https://api.github.com/users/samhatchett/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/labels/enhancement","name":"enhancement","color":"84b6eb"},{"url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/labels/help%20wanted","name":"help wanted","color":"159818"},{"url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/labels/Request%20For%20Input","name":"Request For Input","color":"0052cc"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2015-12-11T21:14:55Z","updated_at":"2015-12-12T22:03:00Z","closed_at":null,"body":"@bemcdonnell did a fantastic thing by submitting his Python wrapper for @michaeltryby's binary file parsing API -- but it looks like it needs some cleanup (some paths are hard-coded for instance) before we can use it within a testing script. \r\n\r\nDoes anybody use python wrapper-type code on a daily basis, or have a deep familiarity? I really am a novice Python-er, but I think I've seen some python come from @mauriziocingi @accumodel @eldemet -- others?"},"comment":{"url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/issues/comments/164196876","html_url":"https://github.com/OpenWaterAnalytics/EPANET/issues/54#issuecomment-164196876","issue_url":"https://api.github.com/repos/OpenWaterAnalytics/EPANET/issues/54","id":164196876,"user":{"login":"bemcdonnell","id":6105134,"avatar_url":"https://avatars.githubusercontent.com/u/6105134?v=3","gravatar_id":"","url":"https://api.github.com/users/bemcdonnell","html_url":"https://github.com/bemcdonnell","followers_url":"https://api.github.com/users/bemcdonnell/followers","following_url":"https://api.github.com/users/bemcdonnell/following{/other_user}","gists_url":"https://api.github.com/users/bemcdonnell/gists{/gist_id}","starred_url":"https://api.github.com/users/bemcdonnell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bemcdonnell/subscriptions","organizations_url":"https://api.github.com/users/bemcdonnell/orgs","repos_url":"https://api.github.com/users/bemcdonnell/repos","events_url":"https://api.github.com/users/bemcdonnell/events{/privacy}","received_events_url":"https://api.github.com/users/bemcdonnell/received_events","type":"User","site_admin":false},"created_at":"2015-12-12T22:03:00Z","updated_at":"2015-12-12T22:03:00Z","body":"@mauriziocingi Thanks for your review.  I have some general comments on the Wrapper as is.\r\n\r\n* Pertaining the the Opaque structure - there is a possibility that as the the API evolves, the contents within the ```struct``` can change which means we would need to maintain new opaque structure entries. This, to me, is redundant. Instead of exposing the values via the opaque structure to the user, I would much rather see these values being provided by a function call for user simplicity. Currently you would call ```get_NetSize``` - which is going to be changed since the general consensus is that there is a better way to handle this.... and I too agree.  \r\n* When I committed this, I noted that I'm not done with the exception handling - It's coming :)\r\n* it is true, the C wrapper functions need not sit inside the object and member functions.  I'd like to hear some more thoughts from others on this.  My preferences is to have these as member functions.  I have some ideas in mind for the stats-testing tool that will use this wrapper.. which is partly the reason I have them as member functions.  Ultimately, as member functions, you do not need to pass the pointer ```enrapi``` structure - it's handled inside the object. I've always preferred this approach. Considering this may be used by python programmers with different levels of experiences, I felt this was the simplest way.  \r\n* In the code I committed, I wanted to give anyone the chance to run it the test THAT the process is working. Therefore, I left my code at and beyond ```if __name__ in \"__main__\":```.  This is only intended to show that values are being read correctly (which it appears to be).  I have no intention of leaving this code in.  Furthermore, when I am going through my debugging phase, I like to instantiate my objects as ```self``` so I can quickly check values in the python shell - personal preference; but thanks for the note. \r\n* With respect the ordered indices of hydraulic components, I believe the output file contents are placed in the same order as the input file contents.  This would need to be confirmed, but during my testing I observed this.  I can't say that would be the same case as how EPANET indexes these items internally, but I imagine Lew would have kept this consistent. \r\n* I agree that the ID references will be a good thing to add to this tool. @michaeltryby? :+1: "}}