Skip to content

Commit

Permalink
resolve almost all comments
Browse files Browse the repository at this point in the history
  • Loading branch information
MartinBelthle committed Dec 9, 2024
1 parent 7e4f32b commit 99381a8
Show file tree
Hide file tree
Showing 15 changed files with 35 additions and 46 deletions.
4 changes: 2 additions & 2 deletions antarest/study/business/areas/renewable_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from pydantic import field_validator

from antarest.core.exceptions import DuplicateRenewableCluster, RenewableClusterConfigNotFound, RenewableClusterNotFound
from antarest.core.model import JSON, LowerCaseStr
from antarest.core.model import JSON
from antarest.study.business.all_optional_meta import all_optional_model, camel_case_model
from antarest.study.business.enum_ignore_case import EnumIgnoreCase
from antarest.study.business.utils import execute_or_add_commands
Expand Down Expand Up @@ -340,7 +340,7 @@ def duplicate_cluster(
study: Study,
area_id: str,
source_id: str,
new_cluster_name: LowerCaseStr,
new_cluster_name: str,
) -> RenewableClusterOutput:
"""
Creates a duplicate cluster within the study area with a new name.
Expand Down
6 changes: 2 additions & 4 deletions antarest/study/business/areas/st_storage_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
STStorageMatrixNotFound,
STStorageNotFound,
)
from antarest.core.model import JSON, LowerCaseStr
from antarest.core.model import JSON
from antarest.core.requests import CaseInsensitiveDict
from antarest.core.serialization import AntaresBaseModel
from antarest.study.business.all_optional_meta import all_optional_model, camel_case_model
Expand Down Expand Up @@ -534,9 +534,7 @@ def delete_storages(
)
execute_or_add_commands(study, file_study, [command], self.storage_service)

def duplicate_cluster(
self, study: Study, area_id: str, source_id: str, new_cluster_name: LowerCaseStr
) -> STStorageOutput:
def duplicate_cluster(self, study: Study, area_id: str, source_id: str, new_cluster_name: str) -> STStorageOutput:
"""
Creates a duplicate cluster within the study area with a new name.
Expand Down
4 changes: 2 additions & 2 deletions antarest/study/business/areas/thermal_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
ThermalClusterNotFound,
WrongMatrixHeightError,
)
from antarest.core.model import JSON, LowerCaseStr
from antarest.core.model import JSON
from antarest.study.business.all_optional_meta import all_optional_model, camel_case_model
from antarest.study.business.utils import execute_or_add_commands
from antarest.study.model import STUDY_VERSION_8_7, Study
Expand Down Expand Up @@ -414,7 +414,7 @@ def duplicate_cluster(
study: Study,
area_id: str,
source_id: str,
new_cluster_name: LowerCaseStr,
new_cluster_name: str,
) -> ThermalClusterOutput:
"""
Creates a duplicate cluster within the study area with a new name.
Expand Down
6 changes: 3 additions & 3 deletions antarest/study/business/binding_constraint_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
MatrixWidthMismatchError,
WrongMatrixHeightError,
)
from antarest.core.model import JSON
from antarest.core.model import JSON, LowerCaseStr
from antarest.core.requests import CaseInsensitiveDict
from antarest.core.serialization import AntaresBaseModel
from antarest.core.utils.string import to_camel_case
Expand Down Expand Up @@ -340,7 +340,7 @@ class ConstraintOutput830(ConstraintOutputBase):


class ConstraintOutput870(ConstraintOutput830):
group: str = DEFAULT_GROUP
group: LowerCaseStr = DEFAULT_GROUP


# WARNING: Do not change the order of the following line, it is used to determine
Expand Down Expand Up @@ -553,7 +553,7 @@ def constraint_model_adapter(constraint: t.Mapping[str, t.Any], study_version: S
constraint_output["filter_year_by_year"] = _filter_year_by_year
constraint_output["filter_synthesis"] = _filter_synthesis
if study_version >= STUDY_VERSION_8_7:
constraint_output["group"] = constraint.get("group", DEFAULT_GROUP).lower()
constraint_output["group"] = constraint.get("group", DEFAULT_GROUP)

# Choose the right model according to the version
adapted_constraint: ConstraintOutput
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,18 @@ def validate_color_rgb(v: t.Any) -> str:
_sub_invalid_chars = re.compile(r"[^a-zA-Z0-9_(),& -]+").sub


def transform_name_to_id(name: str, lower: bool = True) -> str:
def transform_name_to_id(name: str) -> str:
"""
Transform a name into an identifier by replacing consecutive
invalid characters by a single white space, and then whitespaces
are striped from both ends.
invalid characters by a single white space, then whitespaces
are striped from both ends and the id is lowered.
Valid characters are `[a-zA-Z0-9_(),& -]` (including space).
Args:
name: The name to convert.
lower: The flag used to turn the identifier in lower case.
"""
valid_id = _sub_invalid_chars(" ", name).strip()
return valid_id.lower() if lower else valid_id
return _sub_invalid_chars(" ", name).strip().lower()


def validate_id_against_name(name: str) -> str:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from pydantic import field_validator

from antarest.core.model import LowerCaseStr
from antarest.study.storage.rawstudy.model.filesystem.config.field_validators import transform_name_to_id
from antarest.study.storage.rawstudy.model.filesystem.config.model import DistrictSet, FileStudyTreeConfig
from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy
Expand Down Expand Up @@ -49,13 +50,6 @@ class CreateDistrict(ICommand):
output: bool = True
comments: str = ""

@field_validator("name")
def validate_district_name(cls, val: str) -> str:
valid_name = transform_name_to_id(val, lower=False)
if valid_name != val:
raise ValueError("Area name must only contains [a-zA-Z0-9],&,-,_,(,) characters")
return val

def _apply_config(self, study_data: FileStudyTreeConfig) -> Tuple[CommandOutput, Dict[str, Any]]:
district_id = transform_name_to_id(self.name)
if district_id in study_data.sets:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
from pydantic import Field, field_validator

from antarest.study.model import STUDY_VERSION_8_6
from antarest.study.storage.rawstudy.model.filesystem.config.field_validators import transform_name_to_id
from antarest.study.storage.rawstudy.model.filesystem.config.field_validators import (
transform_name_to_id,
validate_id_against_name,
)
from antarest.study.storage.rawstudy.model.filesystem.config.model import Area, FileStudyTreeConfig
from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy
from antarest.study.storage.variantstudy.model.command.common import CommandName, CommandOutput
Expand Down Expand Up @@ -46,10 +49,7 @@ class RemoveSTStorage(ICommand):

@field_validator("storage_id", mode="before")
def validate_cluster_name(cls, val: str) -> str:
to_return = transform_name_to_id(val)
if not to_return:
raise ValueError("Cluster name must only contains [a-zA-Z0-9],&,-,_,(,) characters")
return to_return
return validate_id_against_name(val)

def _apply_config(self, study_data: FileStudyTreeConfig) -> t.Tuple[CommandOutput, t.Dict[str, t.Any]]:
"""
Expand Down
16 changes: 8 additions & 8 deletions antarest/study/web/study_data_blueprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

from antarest.core.config import Config
from antarest.core.jwt import JWTUser
from antarest.core.model import JSON, StudyPermissionType
from antarest.core.model import JSON, LowerCaseStr, StudyPermissionType
from antarest.core.requests import RequestParameters
from antarest.core.utils.utils import sanitize_uuid
from antarest.core.utils.web import APITag
Expand Down Expand Up @@ -1955,7 +1955,7 @@ def create_renewable_cluster(
def update_renewable_cluster(
uuid: str,
area_id: str,
cluster_id: str,
cluster_id: LowerCaseStr,
cluster_data: RenewableClusterInput,
current_user: JWTUser = Depends(auth.get_current_user),
) -> RenewableClusterOutput:
Expand All @@ -1965,7 +1965,7 @@ def update_renewable_cluster(
)
request_params = RequestParameters(user=current_user)
study = study_service.check_study_access(uuid, StudyPermissionType.WRITE, request_params)
return study_service.renewable_manager.update_cluster(study, area_id, cluster_id.lower(), cluster_data)
return study_service.renewable_manager.update_cluster(study, area_id, cluster_id, cluster_data)

@bp.put(
path="/studies/{uuid}/areas/{area_id}/clusters/renewable/{cluster_id}/form",
Expand Down Expand Up @@ -2129,7 +2129,7 @@ def create_thermal_cluster(
def update_thermal_cluster(
uuid: str,
area_id: str,
cluster_id: str,
cluster_id: LowerCaseStr,
cluster_data: ThermalClusterInput,
current_user: JWTUser = Depends(auth.get_current_user),
) -> ThermalClusterOutput:
Expand All @@ -2149,7 +2149,7 @@ def update_thermal_cluster(
)
request_params = RequestParameters(user=current_user)
study = study_service.check_study_access(uuid, StudyPermissionType.WRITE, request_params)
return study_service.thermal_manager.update_cluster(study, area_id, cluster_id.lower(), cluster_data)
return study_service.thermal_manager.update_cluster(study, area_id, cluster_id, cluster_data)

@bp.put(
path="/studies/{uuid}/areas/{area_id}/clusters/thermal/{cluster_id}/form",
Expand Down Expand Up @@ -2556,8 +2556,8 @@ def duplicate_cluster(
uuid: str,
area_id: str,
cluster_type: ClusterType,
source_cluster_id: str,
new_cluster_name: str = Query(..., alias="newName", title="New Cluster Name"),
source_cluster_id: LowerCaseStr,
new_cluster_name: LowerCaseStr = Query(..., alias="newName", title="New Cluster Name"),
current_user: JWTUser = Depends(auth.get_current_user),
) -> t.Union[STStorageOutput, ThermalClusterOutput, RenewableClusterOutput]:
logger.info(
Expand All @@ -2577,6 +2577,6 @@ def duplicate_cluster(
else: # pragma: no cover
raise NotImplementedError(f"Cluster type {cluster_type} not implemented")

return manager.duplicate_cluster(study, area_id, source_cluster_id.lower(), new_cluster_name)
return manager.duplicate_cluster(study, area_id, source_cluster_id, new_cluster_name)

return bp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def test_cascade_update(
"action": "create_cluster",
"args": {
"area_id": area_id,
"cluster_name": transform_name_to_id(cluster_name, lower=False),
"cluster_name": transform_name_to_id(cluster_name),
"parameters": _create_thermal_params(cluster_name),
"prepro": np.random.rand(8760, 6).tolist(),
"modulation": np.random.rand(8760, 4).tolist(),
Expand Down
3 changes: 1 addition & 2 deletions tests/storage/repository/filesystem/config/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ def test_transform_name_to_id__nominal_case(name, expected):

@pytest.mark.parametrize("name", VALID_CHARS)
def test_transform_name_to_id__valid_chars(name):
assert transform_name_to_id(name, lower=True) == name.lower()
assert transform_name_to_id(name, lower=False) == name
assert transform_name_to_id(name) == name.lower()


@pytest.mark.parametrize("name", sorted(set(string.punctuation) - set(VALID_CHARS)))
Expand Down
4 changes: 2 additions & 2 deletions tests/variantstudy/model/command/test_create_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ def test_validate_modulation(self, command_context: CommandContext):
def test_apply(self, empty_study: FileStudy, command_context: CommandContext):
study_path = empty_study.config.study_path
area_name = "DE"
area_id = transform_name_to_id(area_name, lower=True)
area_id = transform_name_to_id(area_name)
cluster_name = "Cluster-1"
cluster_id = transform_name_to_id(cluster_name, lower=True)
cluster_id = transform_name_to_id(cluster_name)

CreateArea(area_name=area_name, command_context=command_context, study_version=STUDY_VERSION_8_8).apply(
empty_study
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext) ->
empty_study.config.version = study_version
study_path = empty_study.config.study_path
area_name = "DE"
area_id = transform_name_to_id(area_name, lower=True)
area_id = transform_name_to_id(area_name)
cluster_name = "Cluster-1"

CreateArea(area_name=area_name, command_context=command_context, study_version=study_version).apply(empty_study)
Expand Down
2 changes: 1 addition & 1 deletion tests/variantstudy/model/command/test_remove_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext) ->
area_name = "Area_name"
area_id = transform_name_to_id(area_name)
cluster_name = "Cluster Name"
cluster_id = transform_name_to_id(cluster_name, lower=False)
cluster_id = transform_name_to_id(cluster_name)

study_version = empty_study.config.version

Expand Down
2 changes: 1 addition & 1 deletion tests/variantstudy/model/command/test_remove_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def test_apply(self, tmpdir: Path, command_context: CommandContext, version: int
study_version = empty_study.config.version

# Create some areas
areas = {transform_name_to_id(area, lower=True): area for area in ["Area_X", "Area_Y", "Area_Z"]}
areas = {transform_name_to_id(area): area for area in ["Area_X", "Area_Y", "Area_Z"]}
for area in areas.values():
output = CreateArea(area_name=area, command_context=command_context, study_version=study_version).apply(
empty_study
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext) ->
area_name = "Area_name"
area_id = transform_name_to_id(area_name)
cluster_name = "Cluster Name"
cluster_id = transform_name_to_id(cluster_name, lower=False)
cluster_id = transform_name_to_id(cluster_name)

output = CreateArea(area_name=area_name, command_context=command_context, study_version=study_version).apply(
empty_study
Expand Down

0 comments on commit 99381a8

Please sign in to comment.