Relationship support #20

chicco785 · 2023-10-11T15:29:25Z

This pr takes the journey to bring relationships to modernpython generator based on work covered in zaphiro-technologies#5.
The code is build on top of PR#16, which is required for build reproducibility (using modern library versioning for python).

Tentative plan:

Treat <cims:stereotype rdf:resource="http://iec.ch/TC57/NonStandard/UML#enumeration"/> classes as Enum.
Remove classes for base types (e.g. in Python is nonsense to have a class for Boolean and Float), especially with zero attributes.
Generate relationships, instead of using strings with IDs to define them. Supporting also Associations (@guillaume-alliander is there any reason why you commented them in current code generator?)

chicco785 · 2023-10-12T07:29:29Z

modernpython/langPack.py

+        )
+        # Datatypes based on primitives are never used in the in memory
+        # representation but only for the schema
+        or class_details["is_a_float"] == True


@guillaume-alliander <cims:stereotype>CIMDatatype</cims:stereotype> are never used during the parsing of a cime file, this because they are just metadata.

For example, ApparentPower is used in datatype of the attribute ACDCConverter.baseS, which is just a float in the class model.

Options:

Given that are never used, we remove them as in this code.

We add to the Fields a metadata called data_type (in a similar fashion to in_profile) so that this metadata is preserved and can be used to extract units and scale factor.

Feelings?

Maybe a tad late - I am of 2 minds. I do like the idea of preserving metadata for future use, because at the end of the day we are trying to be generic, but it might be an acute case of YAGNI. I think that if it low effort it's worth keeping it.

generation of data types instances is done, I only need to attach them to the attributes, on tue i should be able to fix it.

@guillaume-alliander also data type annotation is now in

sonarcloud · 2023-10-17T07:23:05Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
14 Code Smells

No Coverage information
No Duplication information

guillaume-alliander

I saw this PR as ready for review, so I had a look.

A few comments, but the main thing is that right now, no resources files are written when I tried and ran it :)

Also, in PR #21 I split Base.py in a few files, after the comments on pycgmes. There will be rebase needed at some point.

CIMgen.py

modernpython/langPack.py

modernpython/schema_header.py

modernpython/langPack.py

m-mirz · 2023-11-10T10:59:37Z

@chicco785 @guillaume-alliander I've merged #16 .
Could you rebase this PR and let me know when it is ready to be reviewed and merged? I would also appreciate an initial review by guillaume as it is indicated in the Reviewers section.

Besides, let me know if you would like to play a leading / maintainer role in this project or cimpy.
We have implemented an initial version of cimpy for CGMES 3 (based on your cimgen PR) but could use more hands to boost development speed ;-)
See this PR: sogno-platform/cimpy#22

chicco785 · 2023-11-14T14:13:00Z

@chicco785 @guillaume-alliander I've merged #16 . Could you rebase this PR and let me know when it is ready to be reviewed and merged? I would also appreciate an initial review by guillaume as it is indicated in the Reviewers section.

@m-mirz i will rebase pr once @guillaume-alliander one is merged. I have already talked with guillame and i will test some additional changes.

Besides, let me know if you would like to play a leading / maintainer role in this project or cimpy. We have implemented an initial version of cimpy for CGMES 3 (based on your cimgen PR) but could use more hands to boost development speed ;-) See this PR: sogno-platform/cimpy#22

Happy to help, with @MarcoPignati (our CTO) we already discussed this. Being a company the work we put may be limited to our needs, but hopefully this is good enough for you :)

For cimpy we need to understand your plans, in my understanding currently it's not based on the "modern python" and we won't make much use of it in that case :(

Maybe a good idea is to have a call also with @guillaume-alliander and share ideas. My timeline currently is quite bad. What about early January?

m-mirz · 2023-11-15T20:18:31Z

@chicco785 Ok, for the rebase. I will take care of merging.

Regarding the roadmap and maintainer role, I am going to talk to Jonas at alliander next week and let you know about the next steps. Thanks for your availability even if it is only limited :-)

For cimpy, the one thing that we missed most dearly in python was types. So, we would also support the switch to a modern python version even if that means larger / breaking changes in cimpy.

m-mirz · 2023-11-24T09:40:47Z

@guillaume-alliander Could you please review this PR as well?

I have made you maintainer of the project. In case you need more permissions, just let me know.

chicco785 · 2023-11-27T00:09:11Z

I saw this PR as ready for review, so I had a look.

A few comments, but the main thing is that right now, no resources files are written when I tried and ran it :)

Also, in PR #21 I split Base.py in a few files, after the comments on pycgmes. There will be rebase needed at some point.

@guillaume-alliander

rebase completed, not sure about the issue you have that nothing is generated... i am passing these args in vscode:

"--outdir", "./build/CGMES_3.0_modernpython", "--schemadir", "./cgmes_schema/CGMES_3.0.0", "--langdir", "modernpython", "--cgmes_version", "cgmes_v3_0_0"

sonarcloud · 2023-11-27T00:19:30Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
8 Code Smells

No Coverage information
No Duplication information

sonarcloud · 2023-11-27T00:19:30Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
8 Code Smells

No Coverage information
No Duplication information

guillaume-alliander

Looking good, but 2 big things I think:

the alias part in pydantic really worries me. It is I think the root of the 6702 type errors I get in the generated classes :(
the all resources in one file vs 1 file per resource is something that I think should be optional

CIMgen.py

guillaume-alliander · 2023-11-28T10:10:48Z

modernpython/utils/dataclassconfig.py

@@ -8,4 +8,23 @@ class DataclassConfig:  # pylint: disable=too-few-public-methods

    # By default, with pydantic extra arguments given to a dataclass are silently ignored.
    # This matches the default behaviour by failing noisily.
-    extra = "forbid"
+    extra = "ignore"


Interesting. Is there a reason to have ignore instead of forbid? (and then the comment above needs update :) )

Which makes me wonder if it would be possible to have the user give the config. I think both values can be useful for different use cases.

ignore make simpler to create instances out of dictionary generate by other tools, such as SQLAlchemy.

guillaume-alliander · 2023-11-28T10:26:10Z

modernpython/templates/cimpy_class_template.mustache

-    # Type {{dataType}} in CIM  # pylint: disable-next=line-too-long
-    # {{/isAssociationUsed}}{{label}} : {{#setType}}{{dataType}}{{/setType}} = Field({{#setDefault}}{{dataType}}{{/setDefault}}, in_profiles = [{{#attr_origin}}Profile.{{origin}}, {{/attr_origin}}]) {{^isAssociationUsed}}# noqa: E501{{/isAssociationUsed}}
-
+    {{#setNormalizedName}}{{label}}{{/setNormalizedName}} : {{#setType}}{{.}}{{/setType}} = Field({{#setDefault}}{{.}}{{/setDefault}}, in_profiles = [{{#attr_origin}}Profile.{{origin}}, {{/attr_origin}}], {{#setCimDataType}}{{.}}{{/setCimDataType}}alias = "{{label}}")


The alias worries me a bit.

Is it needed?

Aliases in pydantic are not exactly what I expect them to be. They have to be used (when accessing an attribute). The initial name basically does not exist any more, which makes the type checkers very confused.

With BaseModel I am not using them, but if I really need an alias, I create a computed_field which is a basically property returning the value of the field I want.

With dataclass, I am not sure how it works.

I have seen your code comment higher, where I put another comment.

The alias worries me a bit.

Is it needed?

the issue is that several attributes have the same name of the Dataclass they refer to, and pydantic dislike that. (i can reproduce it an report the error)

Aliases in pydantic are not exactly what I expect them to be. They have to be used (when accessing an attribute). The initial name basically does not exist any more, which makes the type checkers very confused.

https://docs.pydantic.dev/latest/api/config/#pydantic.config.ConfigDict.populate_by_name

allows to create instances using either the attribute name or the alias. not sure if this works also for accessing...

maybe using validation_alias could work instead of alias, leaving serialisation unchanged? I will test

With BaseModel I am not using them, but if I really need an alias, I create a computed_field which is a basically property returning the value of the field I want.

With dataclass, I am not sure how it works.

guillaume-alliander · 2023-11-28T10:45:43Z

modernpython/langPack.py

+    attribute = eval(renderer(text))
+    datatype = _compute_data_type(attribute)
+    field_type = datatype
+    default = 'default=None'


In a 197 (I grep'ed :) ) places, we end up with (example from PhaseTapChangerTabular):

phaseTapChangerTable : PhaseTapChangerTable = Field(default=None, ...)

With default None but no Optional[]

guillaume-alliander · 2023-11-28T11:00:23Z

modernpython/langPack.py

+# attributes should never have the same name as a class the may map to
+# Python won't be happy with that, so let's normalize attribute names
+# and leverage aliasing in pydantic to use the cim attribute capitalization
+# during value setting.


I am pretty sure this is a pydantic bug: pydantic/pydantic#8240
Aliases would create more issues than it would solve I think.

Hum, not something that can be fixed easily... But I really do not like the alias way. I'll keep thinking about it.

yeah, it seems that some python changes are needed: pydantic/pydantic#7327 (comment)

if we make work 1 resource 1 file with cyclic references, we could use aliases for the imported class names, and this may do.

yeah, it seems that some python changes are needed: pydantic/pydantic#7327 (comment)

It might end up being fixed! pydantic/pydantic#8243 (comment) Fingers crossed, here.

guillaume-alliander · 2023-11-28T11:02:22Z

modernpython/langPack.py

+                output = chevron.render(**args)
+            file.write(output)
+
+def run_template_schema(version_path, class_details, templates):


We both have a strong opinion on all resources in one file vs. 1 resource per file.

And both are very valid use cases I believe.

Maybe add an option to decide which one to choose?

I don't have a strong opinion:) happy to have a look to make work cyclic reference with 1 resource per file, I didn't manage so far.

modernpython/cimdatatype_header.py

Signed-off-by: Federico M. Facca <[email protected]> use enum for <cims:stereotype rdf:resource="http://iec.ch/TC57/NonStandard/UML#enumeration"/> Signed-off-by: Federico M. Facca <[email protected]> don't generate not used classes Signed-off-by: Federico M. Facca <[email protected]> surface cimdatatype and primitive stereoteypes Signed-off-by: Federico M. Facca <[email protected]> normalize attribute names to avoid issues when pointing to classes Signed-off-by: Federico M. Facca <[email protected]> Delete PositionPoint.py Signed-off-by: Federico M. Facca <[email protected]> generate references to classes * clean support for primitives and cim data types * class importer * cyclic reference validator Signed-off-by: Federico M. Facca <[email protected]> copy util.py Signed-off-by: Federico M. Facca <[email protected]> update dataclass config * support populate by name * support deferred evaluation * support build from attributes * change from forbid to ignore (to be tested also without change) Signed-off-by: Federico M. Facca <[email protected]> spring cleans Signed-off-by: Federico M. Facca <[email protected]> attempt to manage cyclic references across multiple files this is not working unfortunately (today); a similar approach may work in the future when superclass inherited type hints may work as well (probably python 3.13+) Signed-off-by: Federico M. Facca <[email protected]>

Signed-off-by: Federico M. Facca <[email protected]>

Co-authored-by: Guillaume Roger <[email protected]> Signed-off-by: Federico M. Facca <[email protected]>

Signed-off-by: Federico M. Facca <[email protected]>

Signed-off-by: Federico M. Facca <[email protected]> annotate datatype Signed-off-by: Federico M. Facca <[email protected]>

Signed-off-by: Federico M. Facca <[email protected]>

sonarcloud · 2023-12-21T12:42:33Z

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

11 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

chicco785 · 2024-10-16T09:26:10Z

hi @guillaume-alliander, how are you?

Finally we are at the point of our development roadmap where we are really going to invest on a solution to manage cim. my colleague @HUG0-D is starting to scratch the surface in these days.

As first thing today, I am going to check what change during my full immersion on the real time platform, and then let's see what to do with this pr and other potential contributions.

chicco785 · 2024-10-16T12:53:55Z

@m-mirz @guillaume-alliander quite some water under the bridges, let's proceed for simple steps, if you agree:

Treat <cims:stereotype rdf:resource="http://iec.ch/TC57/NonStandard/UML#enumeration"/> classes as Enum.
Remove classes for base types (e.g. in Python is nonsense to have a class for Boolean and Float), especially with zero attributes.

Then:

Move to pydantic 2 (before 3 hits us)
Explore ways to support relationships (I had the idea that I have to walk "manually" the relationship via the mRID string and find in my in memory model the correct object...)

chicco785 · 2024-10-16T12:55:33Z

@m-mirz @guillaume-alliander quite some water under the bridges, let's proceed for simple steps, if you agree:

Treat <cims:stereotype rdf:resource="http://iec.ch/TC57/NonStandard/UML#enumeration"/> classes as Enum.

Remove classes for base types (e.g. in Python is nonsense to have a class for Boolean and Float), especially with zero attributes.

Then:

Move to pydantic 2 (before 3 hits us)

Explore ways to support relationships (I had the idea that I have to walk "manually" the relationship via the mRID string and find in my in memory model the correct object...)

adding to the loop also devs that seem more active lately:
@stv0g @tom-hg57

chicco785 force-pushed the relationship-support branch 6 times, most recently from d8ed051 to ed4327c Compare October 11, 2023 17:07

chicco785 commented Oct 12, 2023

View reviewed changes

chicco785 force-pushed the relationship-support branch 6 times, most recently from 862d7a3 to de18931 Compare October 16, 2023 20:31

chicco785 marked this pull request as ready for review October 16, 2023 20:32

chicco785 force-pushed the relationship-support branch from e25ab16 to 01aa921 Compare October 17, 2023 07:22

guillaume-alliander reviewed Oct 20, 2023

View reviewed changes

m-mirz mentioned this pull request Nov 10, 2023

Moderpython simplification + Attribute NS #21

Merged

chicco785 force-pushed the relationship-support branch 6 times, most recently from 95a6ed6 to 4d47385 Compare November 27, 2023 00:03

chicco785 force-pushed the relationship-support branch 2 times, most recently from 053f04f to 41d41cd Compare November 27, 2023 19:09

guillaume-alliander requested changes Nov 28, 2023

View reviewed changes

chicco785 force-pushed the relationship-support branch from e0b12b0 to 4f268ef Compare November 28, 2023 12:24

chicco785 and others added 10 commits December 21, 2023 13:42

support single file generation for enum and schema

f003774

Signed-off-by: Federico M. Facca <[email protected]>

remove header resolution, uncomment associations

9cb0f1e

Signed-off-by: Federico M. Facca <[email protected]>

Apply suggestions from code review

02cd04e

Co-authored-by: Guillaume Roger <[email protected]> Signed-off-by: Federico M. Facca <[email protected]>

fix import

5f5f9c0

Signed-off-by: Federico M. Facca <[email protected]>

complete rebase

626863f

Signed-off-by: Federico M. Facca <[email protected]>

Update dataclassconfig.py

707c696

Signed-off-by: Federico M. Facca <[email protected]>

generate primitives and datatypes instances

691c5df

Signed-off-by: Federico M. Facca <[email protected]> annotate datatype Signed-off-by: Federico M. Facca <[email protected]>

clean imports

5e0c5a8

Signed-off-by: Federico M. Facca <[email protected]>

clean directory only if flag

84f8d10

Signed-off-by: Federico M. Facca <[email protected]>

chicco785 force-pushed the relationship-support branch from 5a286a7 to 84f8d10 Compare December 21, 2023 12:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Relationship support #20

Relationship support #20

chicco785 commented Oct 11, 2023

chicco785 Oct 12, 2023

guillaume-alliander Oct 20, 2023

chicco785 Nov 27, 2023

chicco785 Nov 27, 2023

sonarcloud bot commented Oct 17, 2023

guillaume-alliander left a comment

m-mirz commented Nov 10, 2023

chicco785 commented Nov 14, 2023

m-mirz commented Nov 15, 2023

m-mirz commented Nov 24, 2023

chicco785 commented Nov 27, 2023

sonarcloud bot commented Nov 27, 2023

sonarcloud bot commented Nov 27, 2023

guillaume-alliander left a comment

guillaume-alliander Nov 28, 2023

chicco785 Nov 28, 2023

guillaume-alliander Nov 28, 2023

guillaume-alliander Nov 28, 2023

chicco785 Nov 28, 2023

guillaume-alliander Nov 28, 2023

guillaume-alliander Nov 28, 2023

guillaume-alliander Nov 28, 2023

chicco785 Nov 28, 2023

chicco785 Nov 28, 2023

guillaume-alliander Nov 30, 2023

guillaume-alliander Nov 28, 2023

chicco785 Nov 28, 2023

sonarcloud bot commented Dec 21, 2023

chicco785 commented Oct 16, 2024

chicco785 commented Oct 16, 2024

chicco785 commented Oct 16, 2024

Relationship support #20

Are you sure you want to change the base?

Relationship support #20

Conversation

chicco785 commented Oct 11, 2023

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

sonarcloud bot commented Oct 17, 2023

guillaume-alliander left a comment

Choose a reason for hiding this comment

m-mirz commented Nov 10, 2023

chicco785 commented Nov 14, 2023

m-mirz commented Nov 15, 2023

m-mirz commented Nov 24, 2023

chicco785 commented Nov 27, 2023

sonarcloud bot commented Nov 27, 2023

sonarcloud bot commented Nov 27, 2023

guillaume-alliander 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 21, 2023

Quality Gate passed

chicco785 commented Oct 16, 2024

chicco785 commented Oct 16, 2024

chicco785 commented Oct 16, 2024