Adapt PSDM to parse simonaMarkovLoad JSON models#1473
Adapt PSDM to parse simonaMarkovLoad JSON models#1473PhilippSchmelter wants to merge 25 commits intodevfrom
Conversation
PhilippSchmelter
requested review from
danielfeismann,
sebastian-peter and
staudtMarius
as code owners
There was a problem hiding this comment.
Here are some first comments. But we should talk about the source again. With the current implementation we split the markov power source into two source classes, while the other power sources are implemented with one class.
Two more things from my side:
- Could you add more javadocs and comments in the code to improve the readability of the code?
- Could you add some information for readTheDocs?
| public BufferedReader initReader(Path filePath) throws FileNotFoundException { | ||
| InputStream inputStream = openInputStream(filePath); | ||
| return new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8), 16384); | ||
| } |
There was a problem hiding this comment.
Is this necessary? I think this could be removed, since we are only using the input stream to read in the JSON file.
| } | ||
|
|
||
| default TimeModel parseTimeModel(JsonNode timeNode) { | ||
| int bucketCount = requireInt(timeNode, "bucket_count"); |
There was a problem hiding this comment.
I think naming the method acquiere... or extract... would be better.
|
|
||
| JsonNode gmmNode = parametersNode.path("gmm"); | ||
| Parameters.GmmParameters gmm = | ||
| gmmNode.isMissingNode() || gmmNode.isNull() || gmmNode.size() == 0 |
There was a problem hiding this comment.
Could be improved by using gmmNode.isEmpty().
| */ | ||
| private static final String LOAD_PROFILE_TIME_SERIES = | ||
| "lpts_(?<profile>[a-zA-Z]{1,11}[0-9]{0,3})"; | ||
| "(?:lpts|markov)_(?<profile>[a-zA-Z]{1,11}[0-9]{0,3})"; |
There was a problem hiding this comment.
This could lead to a problem if someone is using lpts_h0.csv and markov_h0.json. We should add some explanation to the docs that the profile names need to be unique.
2 participants
Closes #1472