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 <federico.facca@zaphiro.ch> use enum for <cims:stereotype rdf:resource="http://iec.ch/TC57/NonStandard/UML#enumeration"/> Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> don't generate not used classes Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> surface cimdatatype and primitive stereoteypes Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> normalize attribute names to avoid issues when pointing to classes Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> Delete PositionPoint.py Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> generate references to classes * clean support for primitives and cim data types * class importer * cyclic reference validator Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> copy util.py Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> 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 <federico.facca@zaphiro.ch> spring cleans Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> 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 <federico.facca@zaphiro.ch>

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

Co-authored-by: Guillaume Roger <61284231+guillaume-alliander@users.noreply.github.com> Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> annotate datatype Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

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 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 <federico.facca@zaphiro.ch>

remove header resolution, uncomment associations

9cb0f1e

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

Apply suggestions from code review

02cd04e

Co-authored-by: Guillaume Roger <61284231+guillaume-alliander@users.noreply.github.com> Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

fix import

5f5f9c0

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

complete rebase

626863f

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

Update dataclassconfig.py

707c696

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

generate primitives and datatypes instances

691c5df

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch> annotate datatype Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

clean imports

5e0c5a8

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

clean directory only if flag

84f8d10

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>

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

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