Skip to content

Adding functions to draw UML diagrams from requirements - #414

Merged
AlexanderLanin merged 4 commits into
eclipse-score:mainfrom
MaximilianSoerenPollak:MaximilianSoerenPollak-add-UML-draw-functions
Feb 21, 2025
Merged

AlexanderLanin merged 4 commits into
eclipse-score:mainfrom
MaximilianSoerenPollak:MaximilianSoerenPollak-add-UML-draw-functions

Conversation

@MaximilianSoerenPollak

Copy link
Copy Markdown
Contributor

Currently draft PR to gather feedback and comments.

The neccessary changes in the Metamodel are going to be introduced by another PR from @hoe-jo


This is the 'first' implementation of these UML diagram functionalities. They are rather more direct than general, but fulfill the current 'wants' that were raised.
However, to allow for future growth and anticipate complexity getting higher as well as having more control over grouping of drawn items, a more general solution should be worked on.

@MaximilianSoerenPollak MaximilianSoerenPollak changed the title Maximilian soeren pollak add uml draw functions Adding functions to draw UML diagramms from requirements Feb 18, 2025
@MaximilianSoerenPollak MaximilianSoerenPollak changed the title Adding functions to draw UML diagramms from requirements Adding functions to draw UML diagrams from requirements Feb 18, 2025
@github-actions

Copy link
Copy Markdown
Contributor

The created documentation from the pull request is available at: docu-html

Added programaticall functionality for UML diagramm drawings.
Adaptations to Metamodel are coming in future PR

Ref: closes eclipse-score#364
@MaximilianSoerenPollak
MaximilianSoerenPollak force-pushed the MaximilianSoerenPollak-add-UML-draw-functions branch from dca1afd to a0dc967 Compare February 19, 2025 09:41
@hoe-jo
hoe-jo marked this pull request as ready for review February 19, 2025 10:41
Removed assets and README as discussed.
Pictures will instead be rendered inside documentation.
Documentation moved to be more user facing.

@AlexanderLanin AlexanderLanin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code is "good enough" for the desired use case. Except the print statement.

I have no idea whether it draws correct images.
And I doubt that it's extendable / maintainable.
Both restrictions are "ok".

Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py
Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
if not needs_inc:
return set()

needs_implements = set(chain(*(needs[id]["implements"] for id in needs_inc)))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting pattern with chain!

I guess it's the same as (pseudocode) set([e for e in needs[id]["implements"] for id in needs_inc])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes exactly, it kind of makes it I guess a bit easier to read as it's not a 'double' list comprehension and just straight up flattens the list it grabs.
Was one of the nicest and simplest ways I found without having another helper func to flatten the list of lists.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure yet which is more readable :D

@MaximilianSoerenPollak MaximilianSoerenPollak Feb 20, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Neither I guess neither is more readable than, just two for loops, but both are faster.

Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
@MaximilianSoerenPollak

MaximilianSoerenPollak commented Feb 20, 2025 •

Copy link
Copy Markdown
Contributor Author

Code is "good enough" for the desired use case. Except the print statement.

I have no idea whether it draws correct images. And I doubt that it's extendable / maintainable. Both restrictions are "ok".

The images are drawn correctly, the only thing @hoe-jo mentioned was that the 'scope' of some of them (for example 'logical interface' being drawn in the same whitebox as the component). Other than that it follows correctly and links correctly.

This however seems to be rather difficult to implement in this Iteration (without just starting from scratch).
I think this would be one of the improvements and requirements of the 'general' implementation then.

Edit:

Agree to the 'doubtful expandability', this will be very difficult to expand upon with any meaningfull big feature.

This approach is to make it work right now, but as mentioned in talks, as soon as new 'sensible' bigger feature comes in it makes sense to spend time to redo this from the ground up (if that is still a thing we want to implement then)

@MaximilianSoerenPollak

MaximilianSoerenPollak commented Feb 20, 2025 •

Copy link
Copy Markdown
Contributor Author

@AlexanderLanin overall question, in this PR I have included a lot of docstrings, some with examples some without.
Do you think, regarding to what we talked about recently and what is mentioned in #416 that this should be reduced?

Edit:

I Guess the only real function that had no example / little to no merit to have a docstring was the gen_alias one, as that literally is a rather easy to understand oneliner. I deleted that, for now will keep the rest, but adapt wording where applicable.

@AlexanderLanin

AlexanderLanin commented Feb 20, 2025 •

Copy link
Copy Markdown
Member

@AlexanderLanin overall question, in this PR I have included a lot of docstrings, some with examples some without. Do you think, regarding to what we talked about recently and what is mentioned in #416 that this should be reduced?

The examples helped. Some of the parameter explanations are better off with renaming the parameters IMHO. E.g. needs -> all_needs.
I'll put #416 on the next daily agenda.

Removed the caching as it's no longer needed with this structure
Remove debug print statement and no longer needed imports

Renamed variables and changed/removed docstrings for more clarity

Ref: closes eclipse-score#364
Comment thread docs/_tooling/extensions/score_draw_uml_funcs/__init__.py Outdated
Converted classes into functions.
Classes no longer needed as caching works without __repr__ overwrite
now.

Ref: closes eclipse-score#364
@MaximilianSoerenPollak
MaximilianSoerenPollak force-pushed the MaximilianSoerenPollak-add-UML-draw-functions branch from 9ee5566 to ab1f18e Compare February 20, 2025 14:25
@AlexanderLanin
AlexanderLanin merged commit ed3128f into eclipse-score:main Feb 21, 2025
@AlexanderLanin AlexanderLanin moved this from In Progress to Done in INF - Infrastructure Community Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants