Skip to content

Use map container properties for process elements#637

Merged
jensschmeink merged 14 commits into
masterfrom
634-use-map-style-container-properties
Feb 2, 2026
Merged

Use map container properties for process elements#637
jensschmeink merged 14 commits into
masterfrom
634-use-map-style-container-properties

Conversation

@jensschmeink
Copy link
Copy Markdown
Collaborator

The new development of container properties for GTlab which now can be handled via names instead of indices are beneficial for usage in python process elements.

As this changes the datamodel an module upgrader is needed aswell to convert old projects to the new structure

@jensschmeink jensschmeink self-assigned this Nov 17, 2025
@jensschmeink jensschmeink linked an issue Nov 17, 2025 that may be closed by this pull request
@jensschmeink jensschmeink marked this pull request as draft November 17, 2025 08:25
@jensschmeink jensschmeink marked this pull request as ready for review November 24, 2025 10:28
@jensschmeink
Copy link
Copy Markdown
Collaborator Author

@marvinoe21 I am not sure when to merge this code but a review would be good to be ready to add it to be prepared for the first version in combination with GTlab 2.1

Copy link
Copy Markdown
Contributor

@rainman110 rainman110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work and it seems to be correct in the most cases.

Please make the upgrade routine more defensive (they would do the wrong things, if the xml is not as expected) and cleanup the code according to my comments.

Comment thread src/module/processcomponents/gtpy_abstractscriptcomponent.cpp Outdated
Comment thread src/module/utilities/gtpy_decorator.h Outdated
Comment thread src/module/utilities/gtpy_decorator.h
Comment thread src/module/utilities/gtpy_decorator.h
Comment thread src/module/utilities/gtpy_decorator.cpp Outdated
Comment thread src/module/utilities/gtpy_moduleupgrader.cpp Outdated
Comment thread src/module/utilities/gtpy_moduleupgrader.cpp Outdated
Comment thread src/module/utilities/gtpy_moduleupgrader.cpp Outdated
Comment thread src/module/utilities/gtpy_moduleupgrader.cpp
Comment thread src/module/utilities/gtpy_moduleupgrader.cpp Outdated
@jensschmeink
Copy link
Copy Markdown
Collaborator Author

@rainman110 Thanks for the review. I worked on it and hope now the code is improved. Please check again if everything is fine now

Copy link
Copy Markdown
Contributor

@rainman110 rainman110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jensschmeink . Thanks for fixing it up.

I have only a few smaller change requests from your fixup and some general remark for the next time then.

Comment thread src/module/utilities/gtpy_decorator.h Outdated
*/

/**
* @brief getPropertyContainerVal
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rephrase a bit like

@brief Returns the member value of a struct inside a property container

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please move the text "In contrast to function above this function does not use the index"

after the @brief section leaving an empty line between brief and the additional text, not in between the parameters.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jensschmeink Please still fix this docstring

Comment thread src/module/utilities/gtpy_decorator.h
Comment thread src/module/utilities/gtpy_moduleupgrader.cpp Outdated
Comment thread src/module/utilities/gtpy_moduleupgrader.cpp Outdated
@jensschmeink
Copy link
Copy Markdown
Collaborator Author

@rainman110 added more changes

Copy link
Copy Markdown
Contributor

@rainman110 rainman110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jensschmeink There was on thing missing from the last code review.

Please rephrase the docstring as suggested and move the explanation (difference to the other function) up after the brief section

Comment thread src/module/utilities/gtpy_decorator.h Outdated
*/

/**
* @brief getPropertyContainerVal
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jensschmeink Please still fix this docstring

@jensschmeink jensschmeink merged commit c5b47e6 into master Feb 2, 2026
1 check passed
@rainman110 rainman110 deleted the 634-use-map-style-container-properties branch February 2, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use map style container properties

2 participants