[FEATURE] Generate component attributes documentation using code generator
Currently, several files are generated by the code generator in https://github.com/PowerGridModel/power-grid-model/tree/main/code_generation .
However, there is component documentation based on that generated code in https://power-grid-model.readthedocs.io/en/stable/user_manual/components.html that is manually created. This documentation can go out of date without anyone noticing, e.g. because default values changing.
This ticket is about generating the component documentation via the code generator.
TODO
- [ ] Combine https://github.com/PowerGridModel/power-grid-model/blob/main/code_generation/data/attribute_classes/input.json and https://github.com/PowerGridModel/power-grid-model/blob/main/code_generation/data/attribute_classes/update.json into single configuration file:
- All attributes in
update.jsonare also present ininput.json - Therefore, the combined configuration file can generate equivalently loaded
input.json-like andupdate.json-like data based on a boolean flagis_updateable: true, e.g.
- All attributes in
{
"name": "input",
"include_guard": "INPUT",
"classes": [
{
"name": "BranchInput",
"base": "BaseInput",
"attributes": [
{
"data_type": "ID",
"names": [
"from_node",
"to_node"
],
"description": "node IDs to which this branch is connected at both sides",
"is_updateable": false
},
{
"data_type": "IntS",
"names": [
"from_status",
"to_status"
],
"description": "whether the branch is connected at each side",
"is_updateable": true
}
]
}
]
}
- [ ] Add fields to both the
input.json(or equivalent) and theoutput.jsonfiles with the long description from https://power-grid-model.readthedocs.io/en/stable/user_manual/components.html- [ ] both
input.jsonandoutput.json- [ ]
unit(-in the docs should benullin the configuration file) - [ ]
long_description
- [ ]
- [ ] only
input.json- [ ]
required - [ ]
valid_values - [ ]
default_value
- [ ]
- [ ] both
- [ ] Add a template to the code generation templates that generates the tables in https://power-grid-model.readthedocs.io/en/stable/user_manual/components.html (but keeps all other content)
TBD
- [ ] How to handle values that are required for some data types but not for others? (e.g.
r0is only required for asymmetric calculations) - [ ] Should we add an
no_docsorexperimentalflag to make sure that attributes-under-construction are not added to the docs? - [ ] How to nicely handle multi-field component names (e.g.:
from_nodeandto_node)- [ ] Proposed: make
"long_description"an array when there are multiple names, just like"names"
- [ ] Proposed: make
- [ ] How to handle hyperlings?
- Example (taken from https://power-grid-model.readthedocs.io/en/stable/user_manual/components.html#generic-power-sensor ):
{hoverxreftooltip}`user_manual/components:Power Sensor Concrete Types`[concrete types](#power-sensor-concrete-types)
- [ ] Proposed: as long as they don't contain
"we can directly keep using them as is
- Example (taken from https://power-grid-model.readthedocs.io/en/stable/user_manual/components.html#generic-power-sensor ):
- [ ] Are we still missing something?
Example
{
"name": "input",
"include_guard": "INPUT",
"classes": [
{
"name": "BranchInput",
"base": "BaseInput",
"attributes": [
{
"data_type": "ID",
"names": [
"from_node",
"to_node"
],
"description": "node IDs to which this branch is connected at both sides",
"is_updateable": false,
"unit": null,
"long_description": [
"ID of node at from-side",
"ID of node at to-side"
],
"required": true,
"valid_values": "a valid node ID",
"default_value": null
},
{
"data_type": "IntS",
"names": [
"from_status",
"to_status"
],
"description": "whether the branch is connected at each side",
"is_updateable": true,
"unit": null,
"long_description": [
"connection status at from-side",
"connection status at to-side"
],
"required": true,
"valid_values": "`0` or `1`",
"default_value": null
}
]
}
]
}
I am not sure about this issue. The logic is pretty complicated. Also in documentation we will explain the mathematics of the components and their required value ranges.
It is for sure not a good first issue.
I am not sure about this issue. The logic is pretty complicated. Also in documentation we will explain the mathematics of the components and their required value ranges.
As long as they just copy-paste the contents of the tables, that should not be too hard, right?
It is for sure not a good first issue.
Creating the jinja template is not hard. If we refine the TBD parts a bit better, I think the same is true for the json part.
In the current documentation we have also many equations to explain the electrical model of certain component.
If you want to generate that one, you have to put the whole equation (in markdown and latex) in json with many escape characters. I really do not think this is easy. Let alone the maintainability.
In the current documentation we have also many equations to explain the electrical model of certain component.
They can remain in the template file in markdown as is, right? only the table generation has to be jinja templated. No need to add
If you want to generate that one, you have to put the whole equation (in markdown and latex) in json with many escape characters. I really do not think this is easy. Let alone the maintainebility.
If you really want to, it's possible to really only generate a file with nothing more than the tables and then in-place include them in the original components.md file. See e.g. https://stackoverflow.com/questions/69939832/include-whole-text-sections-from-other-rst-files-into-this-file
I also have my doubts about this issue. Auto generating this particular documentation seems overly complicated in my opinion.
Of course it can happen that docs are not properly updated, but they should. If you change a default value you should update the docs and as reviewer you should also notice this. Besides that, I don't think many things will change here, only new components might be added, but we won't change default behavior that quickly.
Cfr. offline discussion: We want to have the additional safeguard of extra reviewing round. Might be a tool to ease our lives at a later stage, but it's not a feature we want to automate.
Related: do we want to combine the input.json and update.json configuration files? If so, we can split that from this issue into a new one and close this one
- [ ] Combine https://github.com/PowerGridModel/power-grid-model/blob/main/code_generation/data/attribute_classes/input.json and https://github.com/PowerGridModel/power-grid-model/blob/main/code_generation/data/attribute_classes/update.json into single configuration file:
- All attributes in
update.jsonare also present ininput.json- Therefore, the combined configuration file can generate equivalently loaded
input.json-like andupdate.json-like data based on a boolean flagis_updateable: true, e.g.{ "name": "input", "include_guard": "INPUT", "classes": [ { "name": "BranchInput", "base": "BaseInput", "attributes": [ { "data_type": "ID", "names": [ "from_node", "to_node" ], "description": "node IDs to which this branch is connected at both sides", "is_updateable": false }, { "data_type": "IntS", "names": [ "from_status", "to_status" ], "description": "whether the branch is connected at each side", "is_updateable": true } ] } ] }
Related: do we want to combine the
input.jsonandupdate.jsonconfiguration files? If so, we can split that from this issue into a new one and close this one
Not sure what we would gain from this. Having input and update separate makes is really clear what lives where. Else, you have to scan for the is_updateable flag for each attribute
But now, we have to copy-paste all attributes, their descriptions and properties, which is more duplication and prone to typos. Also, any change requires changes in multiple locations, instead of just one and it's easy to forget what you were updating. I see your point but I'm not sure whether that outweighs the potential downsides.
Op 20 sep 2024 10:22 schreef Peter Salemink @.***>:
Related: do we want to combine the input.json and update.json configuration files? If so, we can split that from this issue into a new one and close this one
Not sure what we would gain from this. Having input and update separate makes is really clear what lives where. Else, you have to scan for the is_updateable flag for each attribute
— Reply to this email directly, view it on GitHubhttps://github.com/PowerGridModel/power-grid-model/issues/692#issuecomment-2363150324, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADUHKF4IIIJHFPWCPCIJRM3ZXPLM5AVCNFSM6AAAAABMQK4N6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRTGE2TAMZSGQ. You are receiving this because you authored the thread.Message ID: @.***>
@mgovers
I am in favor of let developer to specify separately a list of updatable attributes due to three reasons:
- Force to repeat the attributes you want to update has less prone to error than copy-paste the
"is_updateable": trueand forget to change. - To make your proposal, the code-gen will be more complicated than now. We have now a clean way of rendering
jinja2template by just loop all classe definition json files and templates. It is a cross product. - Change the type, name, and description of an attribute is extremely rare. So, the change multiple places argument is not so obvious. We are mainly talking about add new attributes/components.
Following the discussions above I will close this issue for now.