Warn for missing parameters when converting from EPA to MVS format#656
Warn for missing parameters when converting from EPA to MVS format#656Bachibouzouk merged 10 commits intodevfrom
Conversation
| DATA, | ||
| ) | ||
|
|
||
| from multi_vector_simulator.A1_csv_to_json import MissingParameterError |
There was a problem hiding this comment.
should we define those in utils rather than in multi_vector_simulator.A1_csv_to_json @smartie2076, @SabineHaas ?
There was a problem hiding this comment.
You mean the different error types? Would probably be good to define all of them at one place, to also have similar error types and not create a lot of them...?
There was a problem hiding this comment.
They are defined in A1_csv_to_json for the moment, I think it is the only place where they are defined
There was a problem hiding this comment.
Nope, I also defined personalized error types. Not sure where though..
There was a problem hiding this comment.
Ok, then utils would be a good place to define them I think
There was a problem hiding this comment.
Is this for a later PR? Then, do you want to create an issue for it?
There was a problem hiding this comment.
This PR is small enough to add it here
Typically in the server it will not write to files
smartie2076
left a comment
smartie2076
left a comment
There was a problem hiding this comment.
You are right about the fixcost not being included, but I will actually remove them when solving #655. We should communicate with ICOM that this will change.
|
|
||
| ### Added | ||
| - | ||
| - Warning for missing parameter when parsing inputs from epa to mvs (#656) |
There was a problem hiding this comment.
| - Warning for missing parameter when parsing inputs from epa to mvs (#656) | |
| - Warning for missing parameter when parsing inputs from EPA to MVS (#656) |
| myfile.write(json_data) | ||
| myfile.close() | ||
| logging.info( | ||
| "Converted and stored processed simulation data to json: %s", file_path |
There was a problem hiding this comment.
| "Converted and stored processed simulation data to json: %s", file_path | |
| f"Converted and stored processed simulation data to json: {file_path}" |
| if isinstance(folder_path, dict): | ||
| # the folder_path argument is already a dict | ||
| main_parameters = folder_path | ||
| else: | ||
| # load the mvs input json file into a dict | ||
| json_file_path = os.path.join(folder_path, JSON_FNAME) | ||
| with open(json_file_path) as fp: | ||
| main_parameters = json.load(fp) |
There was a problem hiding this comment.
Does this mean that we now can give the parsed json file directly as an argument for the MVS? In that case, what happens if -ext csv or -ext json is given as an argument as well?
There was a problem hiding this comment.
No, this means that you can provide the parse json to the function compare_input_parameters_with_reference
You can already give the parsed json file direcly as an argument of the MVS if you use the server.run_simulation instead of cli.main (this is what gets executed when running mvs_tool). I was not planning to allow already parsed json to command line.
| DATA, | ||
| ) | ||
|
|
||
| from multi_vector_simulator.A1_csv_to_json import MissingParameterError |
There was a problem hiding this comment.
Is this for a later PR? Then, do you want to create an issue for it?
2 participants
Changes proposed in this pull request:
exceptions.pyinmulti_vector_simulator.utilsto gather custom MVS exceptions (Warn for missing parameters when converting from EPA to MVS format #656)multi_vector_simulator.utils.exceptions.py(Warn for missing parameters when converting from EPA to MVS format #656)The following steps were realized, as well (if applies):
black . --exclude docs/)EXECUTE_TESTS_ON=master pytest)For more information on how to contribute check the CONTRIBUTING.md.