Added Link Connection and type helper functions #37
Conversation
|
Updated PR to now cover #18 |
pyswmm/links.py
Outdated
| ... print c1c2.is_conduit() | ||
| >>> True | ||
| """ | ||
| if self._model.getLinkType(self._linkid) is LinkType.conduit: |
There was a problem hiding this comment.
return self._model.getLinkType(self._linkid) is LinkType.conduit
pyswmm/links.py
Outdated
| ... print c1c2.is_orifice() | ||
| >>> False | ||
| """ | ||
| if self._model.getLinkType(self._linkid) is LinkType.orifice: |
There was a problem hiding this comment.
return self._model.getLinkType(self._linkid) is LinkType.orifice
pyswmm/links.py
Outdated
| ... print c1c2.is_weir() | ||
| >>> False | ||
| """ | ||
| if self._model.getLinkType(self._linkid) is LinkType.weir: |
There was a problem hiding this comment.
self._model.getLinkType(self._linkid) is LinkType.weir
pyswmm/links.py
Outdated
| ... print c1c2.is_outlet() | ||
| >>> False | ||
| """ | ||
| if self._model.getLinkType(self._linkid) is LinkType.outlet: |
There was a problem hiding this comment.
self._model.getLinkType(self._linkid) is LinkType.outlet
pyswmm/links.py
Outdated
| return False | ||
|
|
||
| @property | ||
| def link_connections(self): |
There was a problem hiding this comment.
just connections maybe? or does it make sense to prefix with link?
|
@bemcdonnell looking great, I added some comments that are quick changes :-) |
|
@goanpeca, I updated the helper functions and I added nodes.py. |
| # ----------------------------------------------------------------------------- | ||
| """Nodes module for the pythonic interface to SWMM5.""" | ||
|
|
||
| from toolkitapi import * |
There was a problem hiding this comment.
In general using the * on imports is discouraged. its better to be explicit about what gets imported.
There was a problem hiding this comment.
I agree. In the next milestone I plan to address all of these
pyswmm/nodes.py
Outdated
| :rtype: int | ||
|
|
||
| """ | ||
| return self._nNodes |
There was a problem hiding this comment.
Why not add
self._nNodes = self._model.getProjectSize(ObjectType.NODE) directly here?
pyswmm/nodes.py
Outdated
| raise StopIteration() | ||
|
|
||
| @property | ||
| def nodeid(self): |
There was a problem hiding this comment.
node_ inside node object is redundant but id is a reserved word so maybe nid otherwise we would need to use id_, which many libraries do?
Why is there a nodeid in the nodes objetct?
There was a problem hiding this comment.
The way i have set this us is to pass a the ID to instantiate a Node class. So I needed a simple way to get an ID
There was a problem hiding this comment.
It is for the iterator
There was a problem hiding this comment.
Yes but it feels like a private method then.... _nodeid
There was a problem hiding this comment.
That's a nice point, I will make it a private method (both nodes.py and links.py)
| ... 0.0 | ||
|
|
||
| """ | ||
| def __init__(self, model, nodeid): |
There was a problem hiding this comment.
As we move forward, passing the model should be unnecessary cause the Network object or whatever we decide to call it should be aware of the model already.
There was a problem hiding this comment.
I think this is a good idea. We can raise an issue and talk strategy for the next milestone
| def flooding(self): | ||
| """ | ||
| Get Node Results for Flooding Rate. If Simulation is not running | ||
| this method will raise a warning and return 0. |
There was a problem hiding this comment.
Why 0 as opposed to None?
There was a problem hiding this comment.
Because when the simulation has not began, these values are all 0.
Covers issue #36
I also updated the docs a little to alleviate link/links/Links/Link confusion... @michaeltryby