prof_to_cross utility #488
Conversation
Done: * commandline argparse * callable for either mdu of prof* files * converts profloc Todo: * convert profdef * convert profdefxyz
* also added APIdocs * also added first version of end-user usage docs.
…HYDROLIB-core into feat/487_prof_to_cross
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
Should something be added to this file?
| @@ -0,0 +1,545 @@ | |||
| import argparse | |||
There was a problem hiding this comment.
Do we want to put these file in the tools folder or in a sub folder in the tools folder? e.g. tools/converters
There was a problem hiding this comment.
would it be an idea to splitup this file into smaller files?
There was a problem hiding this comment.
The conclusion of last week's discussion was: begin with all tools directly inside the tools directory.
But now that this stis here, let's double check tomorrow (also taking your question below into account).
| @@ -0,0 +1,46 @@ | |||
| import os | |||
There was a problem hiding this comment.
Should the cmd line input (main) also be tested?
There was a problem hiding this comment.
these tests only cover the prof_to_cross & prof_to_cross_from_mdu "public" methods, do we also need to test the other "public" methods?
| if _proftype >= 2: | ||
| _proftype = math.floor(_proftype / 2.0) * 2 | ||
|
|
||
| if _proftype == 1: # Pipe |
There was a problem hiding this comment.
Maybe you can use a dictionary to do this?
_crstype_map = {
1: "circle",
2: "rectangle",
4: "yz",
6: "yz",
100: "yz",
200: "xyz",
}
if _proftype in _crstype_map:
crstype = _crstype_map[_proftype]
else:
raise ValueError(f"Invalid legacy profile type given: TYPE={proftype}")
There was a problem hiding this comment.
I thought about this! But then I thought: YAGNI.
I'll think about it
| str: Equivalent frictionType string value for use in a crsdef.ini. | ||
| """ | ||
|
|
||
| if frctp == 0: |
There was a problem hiding this comment.
Maybe the same refactor as described by _proftype_to_crsdeftype?
|
|
||
| (proftype, crstype) = _proftype_to_crsdeftype(profdef.get("TYPE")) | ||
|
|
||
| crsdata = {} |
There was a problem hiding this comment.
Maybe the same refactor as described by _proftype_to_crsdeftype?
You could also move this to a seperate method.
|
|
||
| crsdata["id"] = f"PROFNR{profdef['PROFNR']}" | ||
| crsdata["type"] = crstype | ||
| if crstype == "circle": |
There was a problem hiding this comment.
Maybe the same refactor as described by _proftype_to_crsdeftype?
You could also move this to a seperate method.
| crslocfile: PathOrStr = None, | ||
| crsdeffile: PathOrStr = None, | ||
| ): | ||
| """The actual converter function for converting legacy profile files |
There was a problem hiding this comment.
does "actual" add value to this description?
There was a problem hiding this comment.
how do we want to make a distinction between the wrapper and this method towards the users in the method description?
There was a problem hiding this comment.
Is the addition in the wrapper function [..], for files listed in an MDU file. not sufficiently clear?
| crslocfile: PathOrStr = None, | ||
| crsdeffile: PathOrStr = None, | ||
| ): | ||
| """Wrapper converter function for converting legacy profile files |
There was a problem hiding this comment.
Do we need to state that it is a wrapper, is that information usefull for the user?
There was a problem hiding this comment.
"Wrapper" can go for me, but it's to stress that this has MDU as input, but for the rest is the same as the actual converter function. Note that "user" is a "developer" here.
Fixes #487