Add coverage for the LID_USAGE section #193
Conversation
|
@asselapathirana I created this PR with your fork. However, your fork needs to be updated with the latest changes from aerispaha:swmmio, and merge conflict need to be resolved before we can move forward with this PR. I think the best way to do this is to include only the getter and setter methods for the Can you update your fork of swmmio accordingly? Thanks so much for contributing! |
|
Will update the code and get back. Thanks! |
|
Will do! :-)
Assela Pathirana,
…On Tue, 2 May 2023 at 21:25, Adam Erispaha ***@***.***> wrote:
@asselapathirana <https://github.com/asselapathirana> I created this PR
with your fork. However, your fork needs to be updated with the latest
changes from aerispaha:swmmio, and merge conflict need to be resolved
before we can move forward with this PR. I think the best way to do this is
to include only the getter and setter methods for the lid_usage inp
section, that you've proposed.
Can you update your fork of swmmio accordingly?
Thanks so much for contributing!
—
Reply to this email directly, view it on GitHub
<#193 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACPWVY7LRDWG4PWMGI4FFTXEFNSNANCNFSM6AAAAAAXTOYAEU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Thanks for this PR, @asselapathirana! I have just a couple minor changes requested - with those resolved, we can get this into the next release.
| for section in self._sections: | ||
| # reformate the [SECTION] to section (and _section_df) | ||
| sect_id = section.translate({ord(i): None for i in '[]'}).lower() | ||
| sect_id_private = '_{}_df'.format(sect_id) | ||
| #print(sect_id_private) |
There was a problem hiding this comment.
Can you remove this commented line?
| INFLOWS: [Node, Constituent, Time Series, Type, Mfactor, Sfactor, Baseline, Pattern] | ||
| LID_USAGE: [Subcatchment, 'LID_Process', Number, Area, Width, InitSat, FromImp, ToPerv, RptFile, DrainTo, FromPerv,] |
There was a problem hiding this comment.
Please replace this line with the following:
LID_USAGE: [Subcatchment, LID_Process, Number, Area, Width, InitSat, FromImp, ToPerv, RptFile, DrainTo, FromPerv,]This edit just removes the extra white space. Thanks!
| return self._lid_usage_df | ||
|
|
||
| @property | ||
| def raingages(self): |
There was a problem hiding this comment.
This raingages getter section is already defined above around line 731. Please remove this property.
| @@ -498,6 +499,20 @@ def headers(self): | |||
|
|
|||
| return self._rpt_section_details | |||
|
|
|||
| @property | |||
| def external_outflow_volume(self): | |||
There was a problem hiding this comment.
This is really cool! It is making think, though, that we need to provide coverage for the whole Flow Routing Continuity section of the rpt file. What do you think?
If you agree, maybe we can tackle that in a separate issue.
Then we could extend that logic to provide coverage for the rest of the rpt sections that follow this structure.
There was a problem hiding this comment.
This could be handled in #194
|
@asselapathirana I'm going to merge these changes, and then I'll address my comments in a separate PR |
Summary
This introduces coverage for the [LID_USAGE] inp section to the
swmmio.inpAPI.Makes progress on #57