feat(api): add outputs #37
Conversation
salemsd
commented
Dec 17, 2024
salemsd
commented
- Added outputs and service to study
- Added unit and integration tests
| @@ -52,6 +53,184 @@ class TestCreateAPI: | |||
| ServiceFactory(api, study_id).create_thermal_service(), | |||
| ServiceFactory(api, study_id).create_renewable_service(), | |||
| ) | |||
| json_output = [ | |||
There was a problem hiding this comment.
Wasn't sure about the json for the outputs in the unit tests. Since it's very large, should I just include the relevant keys (id, archived) or leave it like that
There was a problem hiding this comment.
Indeed we don't need all this. You can just simplify by:
{
"name": "20241217-1115eco-sdqsd",
"type": "economy",
"settings": {},
"archived": False,
}for instance
src/antares/model/study.py
Outdated
|
|
||
| Returns: (output_id, Output) dictionary | ||
| """ | ||
| return self._outputs.copy() |
There was a problem hiding this comment.
Instead of doing a copy what we do is we return a MappingProxyType. This basically does the same thing but we should keep the code coherent (see get_binding_constraints method for instance)
src/antares/model/study.py
Outdated
|
|
||
| Returns: Output with the output_id | ||
| """ | ||
| return self._outputs.get(output_id) |
There was a problem hiding this comment.
Should be crash instead of returning a None ? I don't know
| try: | ||
| response = self._wrapper.get(url) | ||
| outputs_json_list = response.json() | ||
| outputs_list: list[Output] = list() |
There was a problem hiding this comment.
Could be a comprehension list:
outputs_list = [Output(name=output["name"], archived=output["archived"]) for output in outputs_json_list]
tests/integration/test_web_client.py
Outdated
| @@ -499,7 +499,12 @@ def test_creation_lifecycle(self, antares_web: AntaresWebDesktop): | |||
| new_study.get_binding_constraints().keys() | |||
| ) | |||
|
|
|||
| # ===== test run study simulation and wait job completion ====== | |||
| # ===== Test outputs (Part 1) ===== | |||
There was a problem hiding this comment.
Add - before simulation
tests/integration/test_web_client.py
Outdated
|
|
||
| study.read_outputs() | ||
| outputs = study.get_outputs() | ||
| assert len(outputs) > 0 |
There was a problem hiding this comment.
You can check that you have only one and that it's unarchived
| @@ -52,6 +53,184 @@ class TestCreateAPI: | |||
| ServiceFactory(api, study_id).create_thermal_service(), | |||
| ServiceFactory(api, study_id).create_renewable_service(), | |||
| ) | |||
| json_output = [ | |||
There was a problem hiding this comment.
Indeed we don't need all this. You can just simplify by:
{
"name": "20241217-1115eco-sdqsd",
"type": "economy",
"settings": {},
"archived": False,
}for instance
src/antares/model/study.py
Outdated
|
|
||
| Raises: KeyError if it doesn't exist | ||
| """ | ||
| return self._outputs[f"{output_id}"] |
There was a problem hiding this comment.
could just be self._outputs[output_id]
| try: | ||
| response = self._wrapper.get(url) | ||
| outputs_json_list = response.json() | ||
| if isinstance(outputs_json_list, dict): |
There was a problem hiding this comment.
the endpoint is supposed to return a list so I don't see where you could enter this if
There was a problem hiding this comment.
My bad i misread the api doc
| @@ -215,6 +217,15 @@ def test_read_study_api(self): | |||
| mocker.get(renewable_url, json=[]) | |||
| mocker.get(thermal_url, json=[]) | |||
| mocker.get(storage_url, json=[]) | |||
| mocker.get( | |||
| output_url, | |||
There was a problem hiding this comment.
For these 2 tests, you can just return an empty list: mocker.get(output_url, json=[]) as you don't really test the content
| assert len(self.study.get_outputs()) == 2 | ||
| output1 = self.study.get_output(json_output[0].get("name")) | ||
| output2 = self.study.get_output(json_output[1].get("name")) | ||
| assert output1 is not None |
There was a problem hiding this comment.
I think now you can return the checks of nullity
tests/integration/test_web_client.py
Outdated
| output = study.read_outputs()[0] | ||
| outputs = study.get_outputs() | ||
| assert len(outputs) == 1 | ||
| assert outputs.get(output.name).archived is not False |
There was a problem hiding this comment.
Could be simplified in assert outputs.get(output.name).archived I think
