feat(api): handle link matrices #34
Conversation
…ting of the method
…ting of the method
…s who got into utils.py
… unit testing of those methods
… unit testing of those methods
…h Area -> str (area_id)
…orrecting errors due to merge
…orrecting errors due to merge
…orrecting errors due to merge
…orrecting errors due to merge
…orrecting errors due to merge
src/antares/model/study.py
Outdated
| def delete_area(self, area: Area) -> None: | ||
| self._area_service.delete_area(area) | ||
| self._areas.pop(area.id) | ||
| def delete_area(self, area_id: str) -> None: |
There was a problem hiding this comment.
Here I think we should still have an Area as an argument and then pass the id. It would be clearer for the user I believe
|
|
||
| return link_ui | ||
|
|
||
| def read_links(self) -> list[Link]: | ||
| raise NotImplementedError | ||
| def get_parameters(self, area_from_id: str, area_to_id: str) -> pd.DataFrame: |
There was a problem hiding this comment.
We should choose between area_from_id and area_from between the 2 methods. I think we should type area_from_id as it's clearer
| parameters_path = f"input/links/{area_from_id}/{area_to_id}_parameters" | ||
| matrix = get_matrix(self._base_url, self.study_id, self._wrapper, parameters_path) | ||
| except APIError as e: | ||
| raise LinkDownloadError(f"{area_from_id}/{area_to_id}", "parameters", e.message) |
There was a problem hiding this comment.
You should raise from e instead of giving the e.message. see other parts of the code
src/antares/service/base_services.py
Outdated
| @abstractmethod | ||
| def create_parameters(self, series: pd.DataFrame, area_from: str, area_to: str) -> None: | ||
| """ | ||
| Args: |
There was a problem hiding this comment.
Either remove the args or fill them. I think you can remove them
src/antares/service/base_services.py
Outdated
| """ | ||
| Args: | ||
| area_to: | ||
| area_from: |
There was a problem hiding this comment.
Last comment about this unresolved
| ui_response = AreaUiResponse.model_validate(json_ui) | ||
| area_ui = AreaUi.model_validate(ui_response.to_craft()) | ||
|
|
||
| except APIError as e: | ||
| raise AreaUiUpdateError(area.id, e.message) from e | ||
| raise AreaUiUpdateError(area_id, e.message) from e | ||
|
|
||
| return area_ui | ||
|
|
||
| def delete_area(self, area: Area) -> None: |
There was a problem hiding this comment.
The delete area should only get the area_id to be the same as the other methods. The delete inisde the sutdy.py gets an Area but calls the service with the id
|
|
||
| return link_ui | ||
|
|
||
| def read_links(self) -> list[Link]: | ||
| raise NotImplementedError | ||
| def get_parameters(self, area_from: str, area_to_id: str) -> pd.DataFrame: |
There was a problem hiding this comment.
You forgot to rename area_to_id in area_to
| parameters_path = f"input/links/{area_from}/{area_to_id}_parameters" | ||
| matrix = get_matrix(self._base_url, self.study_id, self._wrapper, parameters_path) | ||
| except APIError as e: | ||
| raise LinkDownloadError(f"{area_from}/{area_to_id}", "parameters", e.message) from e |
There was a problem hiding this comment.
You can remove the e.message as now you raise from e. Also remove it inside the exception class
| series_path = f"input/wind/series/wind_{area_id}" | ||
| upload_series(self._base_url, self.study_id, self._wrapper, series, series_path) | ||
| except APIError as e: | ||
| raise MatrixUploadError(area_id, "wind", e.message) |
There was a problem hiding this comment.
you forgot to put back the from e at the end. This goes for the others methods inside this file
| parameters_path = f"input/links/{area_from}/{area_to}_parameters" | ||
| matrix = get_matrix(self._base_url, self.study_id, self._wrapper, parameters_path) | ||
| except APIError as e: | ||
| raise LinkDownloadError(f"{area_from}/{area_to}", "parameters", e.message) from e |
There was a problem hiding this comment.
Last remark:
The exception class should expect area_form and to as parameter and do the area_from / area_to itself. This way a new user won't have to know how it's done to call the exception
No description provided.