chore(group): enforce lower case for item groups #2333

sylvlecl · 2025-02-06T10:52:36Z

Simpler version of #2283
which will affect only groups, not cluster names and identifiers.

Should be merged after #2332

Signed-off-by: Sylvain Leclerc <[email protected]>

MartinBelthle

As we discussed with Florian, we should do something in the run method in case the study is in version 8.6 or 8.7 and the user didn't specified a solver higher or equal to 8.8
Could be done in another PR but it would mean that this doesn't work on it's own

MartinBelthle · 2025-02-07T09:32:41Z

antarest/study/business/areas/st_storage_management.py

-        values = self.model_dump(mode="json", by_alias=False, exclude_none=True)
-        return create_st_storage_config(study_version=study_version, **values)
+
+def _to_properties(version: StudyVersion, creation: STStorageCreation) -> STStoragePropertiesType:


Why didn't you put this as a method of STStorageCreation ? Because you did a method for RenewableCreation and ThermalCreation so we should probably harmonize

MartinBelthle · 2025-02-07T09:45:31Z

antarest/study/storage/rawstudy/model/filesystem/config/field_validators.py

+    if isinstance(name, int):
+        name = str(name)
+    if not isinstance(name, str):
+        ValueError(f"Invalid name '{name}'.")


Sonar Issue: We should add a raise here :)

MartinBelthle · 2025-02-07T09:49:28Z

antarest/study/storage/rawstudy/model/filesystem/root/input/st_storage/clusters/area/list.py

 from antarest.study.storage.rawstudy.model.filesystem.config.model import FileStudyTreeConfig
 from antarest.study.storage.rawstudy.model.filesystem.context import ContextServer
 from antarest.study.storage.rawstudy.model.filesystem.ini_file_node import IniFileNode

+_VALUE_PARSERS = {any_section_option_matcher("group"): LOWER_CASE_PARSER}


Question: Should we put these parsers / serializers in a util file instead of declarating them every time ?

MartinBelthle · 2025-02-07T09:54:21Z

tests/integration/study_data_blueprint/test_renewable.py

@@ -117,7 +117,7 @@ def test_lifecycle(
        fr_solar_pv_props = {
            **DEFAULT_PROPERTIES,
            "name": fr_solar_pv,
-            "group": "Solar PV",
+            "group": "solar pv",


I think here we should keep the MAJ to ensure we can create a cluster with a group in the wrong format.
We also have to change the check after to see that we return the value lowered

MartinBelthle · 2025-02-07T10:40:26Z

antarest/study/storage/variantstudy/model/command/create_cluster.py

+    def validate_model(cls, values: t.Dict[str, t.Any], info: ValidationInfo) -> t.Dict[str, t.Any]:
+        # Validate parameters
+        if isinstance(values["parameters"], dict):
+            parameters = copy.deepcopy(values["parameters"])


Don't think we need to make a copy here

MartinBelthle · 2025-02-07T12:01:10Z

tests/integration/study_data_blueprint/test_st_storage.py

@@ -361,7 +361,7 @@ def test_lifecycle__nominal(
        grand_maison = "Grand'Maison"
        grand_maison_properties = {
            "name": grand_maison,
-            "group": "PSP_closed",
+            "group": "psp_closed",


Same. We could keep the old writing

MartinBelthle · 2025-02-07T12:04:07Z

tests/integration/test_integration_token_end_to_end.py

@@ -116,15 +116,15 @@ def test_nominal_case_of_an_api_user(client: TestClient, admin_access_token: str
                "parameters": {
                    "group": "Gas",
                    "marginal-cost": 98,
-                    "unitCount": 1,


Did you change it because it doesn't work anymore ? If so, it could be an issue as it's kind of a breaking change (even though the scripts don't use this writing)

MartinBelthle · 2025-02-07T12:07:06Z

tests/storage/repository/filesystem/special_node/test_lower_case_nodes.py

+
+    data = {"0": {"name": "BC_1", "group": "GRP_1"}}
+    node.save(data)
+    # Asserts the data is saved correctly


Could be nice to add the same test as above concerning the save of a new group specifically

MartinBelthle · 2025-02-07T12:08:44Z

tests/variantstudy/model/command/test_create_cluster.py

-                "parameters": {"group": "Nuclear", "nominalcapacity": 2400, "unitcount": 2},
-                "prepro": prepro_id,
-                "modulation": modulation_id,
+                "modulation": "acc60faf9e0c47c77ec61c0d522382676102d8c1de1eb4aac1c5bb3a11682ff8",


Why did you change the prepro and modulation ids ? I think it was better before. Also this hash could be random no ?

MartinBelthle · 2025-02-07T12:13:26Z

tests/variantstudy/test_command_factory.py

                "name": "id",
-                "base_filter": "add-all",
+                "filter_items": ["a"],


Why did this change ?

sylvlecl added 21 commits January 30, 2025 16:37

Backport group modifications from fix/lower-case-issues branch

02531fb

Signed-off-by: Sylvain Leclerc <[email protected]>

WIP

cc2b280

Signed-off-by: Sylvain Leclerc <[email protected]>

WIP

d4a2cc9

Signed-off-by: Sylvain Leclerc <[email protected]>

chore(commands): remove obsolete diff feature which hurt evolvability

8911e8d

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): adapt ST storage tests

5a78089

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): fix mypy

b4bbce3

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): add test for parsing v1

dbe9ab7

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tools): remove all variant management tools

5ccbcaa

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): remove unused tests

1cabbf1

Signed-off-by: Sylvain Leclerc <[email protected]>

Merge branch 'dev' into fix/remove-command-diffs

488867b

fix(commands): remove last usage of command extractor

72e276e

Signed-off-by: Sylvain Leclerc <[email protected]>

Merge branch 'fix/remove-command-diffs' into fix/lower-case-group

1b04d8f

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): fix some tests in command factory

a636177

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(versioning): fix issues related to version handling in commands

a7b260b

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): fix constraint on item names

8276ac0

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): fix more tests

ff1e580

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): fix last tests

fb43b36

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(clusters): remove unused property

d08f683

Signed-off-by: Sylvain Leclerc <[email protected]>

feat(ini): allow custom parsers/serializers for ini options

ed637af

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(tests): improve tests

cf7641a

Signed-off-by: Sylvain Leclerc <[email protected]>

fix(typing): stricter typing

8dfaf0e

Signed-off-by: Sylvain Leclerc <[email protected]>

pull-request-size bot added the size/XXL label Feb 6, 2025

sylvlecl added 5 commits February 6, 2025 12:53

fix(typing): remove unions

10173a8

Signed-off-by: Sylvain Leclerc <[email protected]>

Merge remote-tracking branch 'origin/dev' into feat/custom-ini-serde

d5d5e96

Merge remote-tracking branch 'origin/dev' into fix/lower-case-group

368d56f

Signed-off-by: Sylvain Leclerc <[email protected]>

Merge branch 'feat/custom-ini-serde' into fix/lower-case-group

8c898f8

feat(ini): parse and serialize group as lowercase

0f30978

Signed-off-by: Sylvain Leclerc <[email protected]>

sylvlecl marked this pull request as ready for review February 6, 2025 13:35

MartinBelthle requested changes Feb 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(group): enforce lower case for item groups #2333

chore(group): enforce lower case for item groups #2333

sylvlecl commented Feb 6, 2025 •

edited

Loading

MartinBelthle left a comment

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

MartinBelthle Feb 7, 2025

chore(group): enforce lower case for item groups #2333

Are you sure you want to change the base?

chore(group): enforce lower case for item groups #2333

Conversation

sylvlecl commented Feb 6, 2025 • edited Loading

MartinBelthle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sylvlecl commented Feb 6, 2025 •

edited

Loading