XML generation and parsing

[HUG0-D]

The overall goal of this PR is to add a profile parser and a profile writer.

To do so, the Base class is extended with 3 main functions:

• .from_xml(): creates an instance of the class by parsing an xml fragment
• .updates_from_xml(): updates an instance of the class by parsing an xml fragment
• .to_xml() : allows to export, for a specified profile, the python object as an etree.Element.

reader.py leverages .from_xml() and .updates_from_xml() to parse different profile (CGMES or custom) to create a dictionary of objects of the form {"mRID": corresponding_python_object}

writer.py exports the python objects to generate the xml Profiles

This PR builds upon the work of #39

[HUG0-D]

You are correct wrapped_class_comment shoud be used in enum_template. In the same way wrapped_comment should be implemented for enum instances as it is not the case currently in cimgen.py

Concerning your test in test_load_all.py, it tries to instantiate all classes in resource folder. However, classes generated with primitive_template.mustache, dataclass_template.mustache and enum_template.mustache cannot be instantiated.
One of the goals of the previous PR was to avoid creating classes that are then never instantiated.

• Classes such as Integer or Float are therefore instances of the class Primitive.
• Datatype classes such as CurrentFlow are instances of the class CIMDatatype. Theses classes provide information on a value (unit, multiplier, ...).
• Enum classes such as UnitSymbol cannot be instantiated either.

To test these classes the best way is to test their attributes directly.

I fixed the pyright type test error by setting the attribute_class to the name of the class instead of the object itself.
It will make it less straightforward to retrieve the unit, multipler or type of the attribute_class but fixes the issue and simplifies the class_template.mustache

[m-mirz]

@tom-hg57 Can you update your review based on the most recent commits of @HUG0-D ?

[tom-hg57]

The order of imports is checked by ruff in pycgmes!
There are other ruff warnings in reader.py and writer.py + pyright errors.
Please check out pycgmes, include your new generated sources and fix the problems.

[tom-hg57]

In my opinion, Base should only contain the things we need as a base class for the generated classes, as in all the other languages.
At least all after apparent_name is only for import/export from/to xml. These could be moved as static functions to the reader or writer or to a common file for both, maybe xml.py or similar.

[tom-hg57]

Suggested change

	elif class_attribute["is_primitive_attribute"] or class_attribute["is_datatype_attribute"]:
	attr_value = xml_attribute.text
	if self.__dataclass_fields__[attr_name].type == bool:
	attr_value = {"true": True, "false": False}.get(attr_value, None)
	elif class_attribute["is_primitive_attribute"] or class_attribute["is_datatype_attribute"]:
	attr_value = eval(str(xml_attribute.text))
	if self.__dataclass_fields__[attr_name].type == bool:
	assert xml_attribute.text
	attr_value = {"true": True, "false": False}.get(attr_value, None)

[HUG0-D]

attr_value = eval(str(xml_attribute.text)) fails if a string contains -

[tom-hg57]

I'm confused.
At first, this change is not what I've committed. My correct change was:

         elif class_attribute["is_list_attribute"]:
-            attr_value = eval(xml_attribute.text)
+            attr_value = eval(str(xml_attribute.text))
         elif class_attribute["is_primitive_attribute"] or class_attribute["is_datatype_attribute"]:
+            assert xml_attribute.text
             attr_value = xml_attribute.text

But maybe that doesn't change your problem with attr_value = eval(str(xml_attribute.text)).
Pyright says that xml_attribute.text is str | None, but eval doesn' take a None.
Maybe we could include assert xml_attribute.text like in the datatype case:

         elif class_attribute["is_list_attribute"]:
+            assert xml_attribute.text
             attr_value = eval(xml_attribute.text)
        elif class_attribute["is_primitive_attribute"] or class_attribute["is_datatype_attribute"]:
+            assert xml_attribute.text
             attr_value = xml_attribute.text

But if xml_attribute.text is not None then it's a string and str() shouldn't change the value. If eval() has a problem with "-" that would also be a problem in a case with a list attribute that has a "-".
BTW: What is the purpose of this eval()?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[tom-hg57]

It is not guaranted that this is a UUID. I know you don't try to convert the string to a UUID. But the name of the variable is misleading.

[HUG0-D]

I will change it to rdfid

[tom-hg57]

Maybe it would be better if all are optional (float | None, int | None ...), so the writer could avoid to give them out. Currently your writer writes all primitive values to xml even if they were not in the input data of the reader.
My chevron_writer avoids this by checking the value. But that leads to another problem: it skips 0 and false values, because it could not distinguish between no value and 0/false.

[HUG0-D]

Yes, at first I implemented something similar to you and faced the same issue.
Having None by default would solve the problem for the writer but could this create issues when using the objects for other applications ?

[tom-hg57]

These attributes are optional. When the reader reads a CIM file the result should show which attribute was in the file and which was miising. And the writer should only write attributes which are really set.

[chicco785]

In my opinion, Base should only contain the things we need as a base class for the generated classes, as in all the other languages.
At least all after apparent_name is only for import/export from/to xml. These could be moved as static functions to the reader or writer or to a common file for both, maybe xml.py or similar.

@tom-hg57 will you still keep such static functions in this repo? in my opinion, writer and reader should be part of the code base. changes to the generated code may always required updates to them, making very hard to maintain them otherwise.

[m-mirz]

In my opinion, Base should only contain the things we need as a base class for the generated classes, as in all the other languages.
At least all after apparent_name is only for import/export from/to xml. These could be moved as static functions to the reader or writer or to a common file for both, maybe xml.py or similar.

@tom-hg57 will you still keep such static functions in this repo? in my opinion, writer and reader should be part of the code base. changes to the generated code may always required updates to them, making very hard to maintain them otherwise.

I guess we need to ask ourselves where we draw the line between the projects. Do we want to copy all code from cimpy, pycgmes etc. into cimgen? If not, what do we exclude and why? Where do we test code if most of it is already included in cimgen?
Having Reader and Writer code in cimgen and cimpy, pycgmes… could also be very confusing.

Another direction could be to include cimgen as a submodule in these projects or to include a script that checks out a specific version of cimgen, generates the code and copies it to the right folders. We need to find a structure that facilitates development and is not too confusing for users.

[tom-hg57]

In my opinion, Base should only contain the things we need as a base class for the generated classes, as in all the other languages.
At least all after apparent_name is only for import/export from/to xml. These could be moved as static functions to the reader or writer or to a common file for both, maybe xml.py or similar.

@tom-hg57 will you still keep such static functions in this repo? in my opinion, writer and reader should be part of the code base. changes to the generated code may always required updates to them, making very hard to maintain them otherwise.

I guess we need to ask ourselves where we draw the line between the projects. Do we want to copy all code from cimpy, pycgmes etc. into cimgen? If not, what do we exclude and why? Where do we test code if most of it is already included in cimgen? Having Reader and Writer code in cimgen and cimpy, pycgmes… could also be very confusing.

Another direction could be to include cimgen as a submodule in these projects or to include a script that checks out a specific version of cimgen, generates the code and copies it to the right folders. We need to find a structure that facilitates development and is not too confusing for users.

In think, reader and writer should be part of cimgen. On the other hand, I test the generated code + reader/writer with pytest in pycgmes. So Markus idea to automatically take over the cimgen changes to pycgmes is not bad. There we have github workflows including pytest.

I started the discussion with the question if it would be better to take the xml code out of the base class. So we had a clean class hierarchy without lxml dependency: only Base , generated code and some handwritten primitive classes. We could put reader/writer stuff apart from this, maybe in another directory. Or really into pycgmes? So I change my opinion, if we establish the automatic take-over to pycgmes.

But we should do this not before this PR is merged. I will test the last changes in the next days.

[chicco785]

In my opinion, Base should only contain the things we need as a base class for the generated classes, as in all the other languages.
At least all after apparent_name is only for import/export from/to xml. These could be moved as static functions to the reader or writer or to a common file for both, maybe xml.py or similar.

@tom-hg57 will you still keep such static functions in this repo? in my opinion, writer and reader should be part of the code base. changes to the generated code may always required updates to them, making very hard to maintain them otherwise.

I guess we need to ask ourselves where we draw the line between the projects. Do we want to copy all code from cimpy, pycgmes etc. into cimgen? If not, what do we exclude and why? Where do we test code if most of it is already included in cimgen? Having Reader and Writer code in cimgen and cimpy, pycgmes… could also be very confusing.

Another direction could be to include cimgen as a submodule in these projects or to include a script that checks out a specific version of cimgen, generates the code and copies it to the right folders. We need to find a structure that facilitates development and is not too confusing for users.

At least in the case of python (then we can see case by case), what I think it would make sense is that cimgen generated code is released as packages that other projects import, and where as much as possible, interfaces expose by cimgen generated code stays unchanged so that impact on other projects are minimal.

Project like cimpy (or pycgmes) should provide additional functionalities and glue logic that is too complex to generate. For example, managing multiple version of cim, validation rules, etc. This would reduce duplications and compatibility issues among projects building on top of cimgen generated code. Which is crucial I believe for avoiding dispersion: the community is already small, duplicates, complexity in version management, ect will end up in making it's long term life complicated.

True is, for example, that is more than one year that cimgen supports 3.0, but cimpy is still with the old code.

[chicco785]

To better detail (and I know this is ok for python and javascript and may not be for other languages):

• include a release workflow in the repo.
• publish packages in relevant registries.
• update dependent projects to import versioned packages from the registries.

This would simplify maintenance of dependent projects. A project is not updated? Builds will still work importing the correct package version. You don't have time to immediately update the project? Same! You have dependabot in your repo and the update is not causing breaking changes? You can automatically import the new version.

[chicco785]

A consideration about readers and writers: In several languages (and to some extent this is true also for pydantic - given that json parsing and serilisation is native), you get them for free if you properly annotate the generated code. For example this is the case for golang (and we have a branch for that, which was never opened as PR because we were not sure it was of interest: https://github.com/zaphiro-technologies/cimgen/tree/golang/golang). If we assume that no serialisation / parsing functionality is ever covered in cimgen, doing a parser in some languages will become almost an impossible work.

package classes

/*
Generated from the CGMES 3 files via cimgen: https://github.com/sogno-platform/cimgen
*/

//nolint:all
type Analog struct {
	Measurement
	/*
	   Analog represents an analog Measurement.

	   positiveFlowIn: If true then this measurement is an active power, reactive power or current with the convention that
	     a positive value measured at the Terminal means power is flowing into the related
	     PowerSystemResource.
	   AnalogValues: The values connected to this measurement.
	   LimitSets: A measurement may have zero or more limit ranges defined for it.
	*/

	PositiveFlowIn  *bool       `xml:"http://iec.ch/TC57/CIM100# Analog.positiveFlowIn"`
	AnalogValuesIds []Reference `xml:"http://iec.ch/TC57/CIM100# Analog.AnalogValues"`
	LimitSetsIds    []Reference `xml:"http://iec.ch/TC57/CIM100# Analog.LimitSets"`
}

What do you think about organising a call to agree on the matter, possible involving also alliander (pycgmes)?

[tom-hg57]

In branch https://github.com/tom-hg57/pycgmes/tree/try-cimgen-xml-generation-and-parsing I committed some fixes for ruff/pyright warnings and added example programs for your reader and writer.
In my tests I found only two real problems:

1. The reader doesn't read TopologicalNodes from TopologicalIsland:

  <cim:TopologicalIsland rdf:ID="N">
    <cim:IdentifiedObject.name>N</cim:IdentifiedObject.name>
    <cim:TopologicalIsland.TopologicalNodes rdf:resource="#N0" />
    <cim:TopologicalIsland.TopologicalNodes rdf:resource="#N1" />
  </cim:TopologicalIsland>

2. The writer writes a list not correctly (see examples/writer_example.py):

  <cim:TopologicalIsland rdf:ID="N">
    <cim:IdentifiedObject.name>N</cim:IdentifiedObject.name>
    <cim:TopologicalIsland.TopologicalNodes>['N0', 'N1']</cim:TopologicalIsland.TopologicalNodes>
  </cim:TopologicalIsland>

Apart from that, the reader and writer work correctly. :)

The two problems should now be solved.

The reader is okay now. But the writer isn't:

  <cim:TopologicalIsland rdf:ID="_N">
    <cim:IdentifiedObject.name>N</cim:IdentifiedObject.name>
    <cim:TopologicalIsland.TopologicalNodes>N0</cim:TopologicalIsland.TopologicalNodes>
    <cim:TopologicalIsland.TopologicalNodes>N1</cim:TopologicalIsland.TopologicalNodes>
  </cim:TopologicalIsland>

In addition to this added a leading underscore in the rdf ID to follow rdf standards.

I don't think that this is a good idea. If you read and write a CIM file the rdf:IDs shouldn't change.

[tom-hg57]

I know most of the changes are only formatting or similar but ruff/pyright are complaining about this. Everytime I test your code with pycgmes I have to change these first.

Why don't you use pycgmes yourselve? The checks and unit tests help to find programming errors.

[tom-hg57]

Suggested change

	infos = dict()
	infos = {}

[tom-hg57]

Suggested change

	if key.endswith("ID") or key.endswith("about"):
	if key.endswith(("ID", "about")):

[tom-hg57]

Suggested change

	if self.__dataclass_fields__[attr_name].type == bool:
	attr_value = {"true": True, "false": False}.get(attr_value, None)
	assert attr_value is not None
	if self.__dataclass_fields__[attr_name].type is bool:
	attr_value = {"true": True, "false": False}.get(attr_value)

[tom-hg57]

from pydantic import Field
from pydantic.dataclasses import dataclass

from ..resources.types.Currency import Currency
from ..resources.types.UnitMultiplier import UnitMultiplier
from ..resources.types.UnitSymbol import UnitSymbol
from .config import cgmes_resource_config
from .constants import NAMESPACES
from .profile import BaseProfile

[tom-hg57]

Suggested change

	self.import_result["meta_info"] = dict(namespaces=self._get_namespaces(xml_files[0]), urls={})
	self.import_result["meta_info"] = {"namespaces": self._get_namespaces(xml_files[0]), "urls": {}}

[tom-hg57]

Suggested change

	custom_profiles: list[BaseProfile] = [],
	custom_namespaces: dict[str, str] = {},
	custom_profiles: list[BaseProfile] | None = None,
	custom_namespaces: dict[str, str] | None = None,

[tom-hg57]

Suggested change

	"""
	"""
	custom_namespaces = custom_namespaces or {}
	custom_profiles = custom_profiles or []

[tom-hg57]

Suggested change

	self, profile: BaseProfile, model_id: str, custom_namespaces: dict[str, str] = {}
	self, profile: BaseProfile, model_id: str, custom_namespaces: dict[str, str] | None = None

[tom-hg57]

Suggested change

	"""
	"""
	custom_namespaces = custom_namespaces or {}

[tom-hg57]

Suggested change

	if count > 0:
	output = etree.ElementTree(root)
	else:
	output = None
	output = etree.ElementTree(root) if count > 0 else None

[m-mirz]

In my opinion, Base should only contain the things we need as a base class for the generated classes, as in all the other languages.
At least all after apparent_name is only for import/export from/to xml. These could be moved as static functions to the reader or writer or to a common file for both, maybe xml.py or similar.

@tom-hg57 will you still keep such static functions in this repo? in my opinion, writer and reader should be part of the code base. changes to the generated code may always required updates to them, making very hard to maintain them otherwise.

I guess we need to ask ourselves where we draw the line between the projects. Do we want to copy all code from cimpy, pycgmes etc. into cimgen? If not, what do we exclude and why? Where do we test code if most of it is already included in cimgen? Having Reader and Writer code in cimgen and cimpy, pycgmes… could also be very confusing.
Another direction could be to include cimgen as a submodule in these projects or to include a script that checks out a specific version of cimgen, generates the code and copies it to the right folders. We need to find a structure that facilitates development and is not too confusing for users.

At least in the case of python (then we can see case by case), what I think it would make sense is that cimgen generated code is released as packages that other projects import, and where as much as possible, interfaces expose by cimgen generated code stays unchanged so that impact on other projects are minimal.

Project like cimpy (or pycgmes) should provide additional functionalities and glue logic that is too complex to generate. For example, managing multiple version of cim, validation rules, etc. This would reduce duplications and compatibility issues among projects building on top of cimgen generated code. Which is crucial I believe for avoiding dispersion: the community is already small, duplicates, complexity in version management, ect will end up in making it's long term life complicated.

True is, for example, that is more than one year that cimgen supports 3.0, but cimpy is still with the old code.

What you are suggesting is going to have a big impact not only on cimpy but also on pycgmes. In order to avoid code duplication, we would need to remove code from cimpy, pycgmes, etc. For cimpy, we can decide to do it this way. For pycgmes, we would need to align with alliander.

Pulling reader and writer functionality into the cimgen repo means also adding the associated tests into cimgen. Not only for python but also for other languages. I am not opposed to this idea but before we start something like this, we should have a clear idea of what goes where and involve relevant stakeholders.

So, I think it is indeed a good idea to organize a call to discuss this topic and include alliander as you suggest.

Regarding CGMES 3.0, I believe that it is just a matter of focus. We have included CGMES 3.0 in libcimpp and we are also working with CGMES 3.0 in Java. For python and CGMES 3.0, there is modernpython and pycgmes. Since you have decided to create these alternatives rather than contributing to cimpy, we do not feel that it is urgent to increase development speed on the python side. We just implement what is required for our purposes when we need it rather than trying to compete with pycgmes.
In my opinion, it is much more important to facilitate the work with CGMES in other languages rather than pushing two very similar things... Python is not the best choice for all use cases.

[m-mirz]

To better detail (and I know this is ok for python and javascript and may not be for other languages):

* include a release workflow in the repo.

* publish packages in relevant registries.

* update dependent projects to import versioned packages from the registries.

This would simplify maintenance of dependent projects. A project is not updated? Builds will still work importing the correct package version. You don't have time to immediately update the project? Same! You have dependabot in your repo and the update is not causing breaking changes? You can automatically import the new version.

I agree. These are all good suggestions. You do not have to convince me ;-) We are lacking the time to implement these points but I would be happy to merge PRs adding these features.

[m-mirz]

A consideration about readers and writers: In several languages (and to some extent this is true also for pydantic - given that json parsing and serilisation is native), you get them for free if you properly annotate the generated code. For example this is the case for golang (and we have a branch for that, which was never opened as PR because we were not sure it was of interest: https://github.com/zaphiro-technologies/cimgen/tree/golang/golang). If we assume that no serialisation / parsing functionality is ever covered in cimgen, doing a parser in some languages will become almost an impossible work.

package classes

/*
Generated from the CGMES 3 files via cimgen: https://github.com/sogno-platform/cimgen
*/

//nolint:all
type Analog struct {
	Measurement
	/*
	   Analog represents an analog Measurement.

	   positiveFlowIn: If true then this measurement is an active power, reactive power or current with the convention that
	     a positive value measured at the Terminal means power is flowing into the related
	     PowerSystemResource.
	   AnalogValues: The values connected to this measurement.
	   LimitSets: A measurement may have zero or more limit ranges defined for it.
	*/

	PositiveFlowIn  *bool       `xml:"http://iec.ch/TC57/CIM100# Analog.positiveFlowIn"`
	AnalogValuesIds []Reference `xml:"http://iec.ch/TC57/CIM100# Analog.AnalogValues"`
	LimitSetsIds    []Reference `xml:"http://iec.ch/TC57/CIM100# Analog.LimitSets"`
}

What do you think about organising a call to agree on the matter, possible involving also alliander (pycgmes)?

Of course we would include a PR going in this direction. We are not dogmatic about the reader/writer part. If the reader/writer does not require additional code and we are not duplicating code that is already included in another repo (e.g. cimpy, pycgmes), it is a no-brainer to accept this PR :-)

@tom-hg57 already convinced me some weeks ago to move code from libcimpp into cimgen (not reader/writer related but "base" classes). If there are good reasons to change the approach, we are open for discussion.

I would be happy to join a meeting to discuss the general direction.

[chicco785]

I agree. These are all good suggestions. You do not have to convince me ;-) We are lacking the time to implement these points but I would be happy to merge PRs adding these features.

Of course, we are happy to open pr to publish libraries for python in a public registry to start with, then we can see if that seems a good way and understand how to do for the rest

[chicco785]

@tom-hg57 @m-mirz @guillaume-alliander

here is a doodle: https://doodle.com/meeting/organize/id/en7o1PWa

(I am posting here, since I am missing Markus email)