adding --uml-no-inline-groupings-from #872

kirankotari · 2023-07-08T22:17:42Z

When we use --uml-inline-grouping flag we can expect to ignore base modules like tailf-ncs etc, in this PR we are added new flags.

Command:

pyang -f uml l3vpn.yang --path=/tmp/.ncs_uml --uml-inline-groupings --uml-skip-module=tailf-ncs --uml-output-directory=.  --uml-no=module,import,annotation  --uml-add-legend 2> /dev/null

--uml-add-legend adds filenames from shared grouping - Note: not skipping any module for this example
Here is the legend plantUML code generated.

legend
Grouping data pulled from
  l3vpn.yang
  tailf-ncs-services.yang
  tailf-ncs-log.yang
endlegend

--uml-skip-module=tailf-ncs with --uml-inline-groupings, inline grouping adds all the grouping uses in the yang including the base service-data, plan-date and more, in the new flag we can skip entire module from inline-grouping.
Here the skip modules are kept in the {uses} as before --uml-inline-groupings flag.

Image before the --uml-skip-module

Image with --uml-skip-module

nkhancock · 2023-07-10T12:02:46Z

pyang/__init__.py

@@ -1,4 +1,4 @@
 """The pyang library for parsing, validating, and converting YANG modules"""

-__version__ = '2.5.3'
+__version__ = '2.5.4'


I suggest that this decision is left to the repository owners.

nkhancock · 2023-07-10T12:07:27Z

pyang/plugins/uml.py

+                                 dest="uml_skip_module",
+                                 action="append",
+                                 default=[],
+                                 metavar="SKIP MODULE",


Why not just use the default metavar, i.e., UML_SKIP_MODULE to indicate the context of the UML plugin; or at least SKIP_MODULE.

Suggested change

metavar="SKIP MODULE",

nkhancock · 2023-07-10T13:22:54Z

pyang/plugins/uml.py

@@ -93,6 +93,16 @@ def add_opts(self, optparser):
            optparse.make_option("--uml-filter-file",
                                 dest="uml_filter_file",
                                 help="NOT IMPLEMENTED: Only paths in the filter file will be included in the diagram"),
+            optparse.make_option("--uml-skip-module",


The --uml-inline-groupings option is then not applied to those uses statements for groupings in the list of modules specified in --uml-skip-module? Correct?

Is this intended to only be applicable when the --uml-inline-groupings global option with the --uml-skip-module option is selected?

If so then the name of this option is too general and is thus misleading and should be improved to make it clearer, e.g., something like --uml-no-inline-groupings-from.

Also if --uml-inline-groupings and --uml-skip-module are not specified wouldn't it be expected that if a legend is specified, then the modules of the groupings should be listed as when --uml-inline-groupings=true and --uml-skip-module is not empty?

The --uml-inline-groupings option is then not applied to those uses statements for groupings in the list of modules specified in --uml-skip-module? Correct?

No, and I agree to the 3 point to switch to --uml-no-inline-groupings-from

Is this intended to only be applicable when the --uml-inline-groupings global option with the --uml-skip-module option is selected?

Yes

If so then the name of this option is to general and is thus misleading and should be improved to make it clearer, e.g., something like --uml-no-inline-groupings-from.

I will adopt to this option

Also if --uml-inline-groupings and --uml-skip-module are not specified wouldn't it be expected that if a legend is specified, then the modules of the groupings should be listed as when --uml-inline-groupings=true and --uml-skip-module is not empty?

You point is true, but if I don't use --uml-inline-groupings the uml is not traversing to it's groupings (other modules) so far. Now I see based on your comment the legend can be improved to touch the submodule too to keep in generic not just for --uml-inline-groupings

nkhancock · 2023-07-10T13:24:02Z

pyang/plugins/uml.py

+                                 action="append",
+                                 default=[],
+                                 metavar="SKIP MODULE",
+                                 help="skips given modules, i.e., --uml-skip-module=tailf-ncs"),


Text should be improved to make it clearer what the actual skipping of modules means. See my comment on line 96.

Also avoid referencing specific modules that may not be available to everyone, e.g., tailf-ncs.

@nkhancock do you have any common example for this?

@nkhancock do you have any common example for this?

A possible candidate could be ietf-yang-push. This has internal groupings and also uses an external grouping from ietf-yang-patch.

nkhancock · 2023-07-10T13:28:03Z

pyang/plugins/uml.py

+                                 default=[],
+                                 metavar="SKIP MODULE",
+                                 help="skips given modules, i.e., --uml-skip-module=tailf-ncs"),
+            optparse.make_option("--uml-add-legend",


The name of this option implies that the user could define or control the rendering of a legend in the diagram, whereas this legend is specific to the --uml-skip-module option.

It also needs to be considered how such an option could co-exist with a possible new option to render a user-specified legend going forward.

Maybe consider using a note instead.

Initially I tried note, which is mapping to the last class (It needed more logic to map to right class etc) let me revisit and add the changes.

@nkhancock
QQ: Shall I pull out the changes of --uml-add-legend and open a new PR when it's ready?

Shall I pull out the changes of --uml-add-legend and open a new PR when it's ready?

That's up to you. If the changes are completely independent, maybe separate PRs would make it easier to review. It's basically up to you.

pyang/plugins/uml.py

nkhancock · 2023-07-10T13:36:32Z

pyang/plugins/uml.py

@@ -591,7 +615,12 @@ def emit_module_header(self, module, fd):
    def emit_module_class(self, module, fd):
        fd.write('class \"%s\" as %s << (M, #33CCFF) module>> \n' %(self.full_display_path(module), self.full_path(module)))

-
+    def emit_uml_legend(self, module, fd):


The name of this function is too general for the function it performs. It is specific to the --uml-skip-modules option.

To allow possible future support for a user to define a legend with a given text, this function should just accept a text to be shown within the legend box and a new function specific to --uml-skip-modules, e.g., emit_uml_skip_modules_legend could be defined that creates the required text for the skip module legend and calls the general function.

pyang/plugins/uml.py

nkhancock · 2023-07-10T15:10:27Z

pyang/plugins/uml.py

@@ -591,7 +615,12 @@ def emit_module_header(self, module, fd):
    def emit_module_class(self, module, fd):
        fd.write('class \"%s\" as %s << (M, #33CCFF) module>> \n' %(self.full_display_path(module), self.full_path(module)))

-
+    def emit_uml_legend(self, module, fd):
+        fd.write('legend \n')


Should a legend be written if there are no files/modules to list, i.e., only when self.ctx_grp_files is not empty? If I don't specify either --uml-inline-groupings or --uml-skip-module, but specify --uml-add-legend, then a legend is displayed without any files listed.

Also if --uml-inline-groupings and --uml-skip-module are not specified wouldn't it be expected that if a legend is specified, then the modules of the groupings should be listed as when --uml-inline-groupings=true and --uml-skip-module is not empty?

kirankotari · 2023-07-10T20:25:47Z

@nkhancock Thanks for reviewing it. I will do update and improve the content.

updating `--uml-add-legend` help Co-authored-by: Nick Hancock <nick.hancock@adtran.com>

updating to module Co-authored-by: Nick Hancock <nick.hancock@adtran.com>

legend msg update Co-authored-by: Nick Hancock <nick.hancock@adtran.com>

nkhancock · 2023-07-12T12:51:05Z

pyang/plugins/uml.py

@@ -62,6 +62,11 @@ def add_opts(self, optparser):
                                 dest="uml_inline",
                                 default =False,
                                 help="Inline groupings where they are used."),


Maybe a note could be added to this text to indicate that groupings from specific modules can be omitted from this option through use of the --uml-no-inline-groupings-from option.

nkhancock · 2023-07-12T12:52:12Z

pyang/plugins/uml.py

@@ -62,6 +62,11 @@ def add_opts(self, optparser):
                                 dest="uml_inline",
                                 default =False,
                                 help="Inline groupings where they are used."),
+            optparse.make_option("--uml-no-inline-groupings-from",


Although suggested by myself, I am not totally convinced by this name.
Maybe others have better suggestions.

adding --uml-skip-module, --uml-add-legend

8dc80f7

nkhancock suggested changes Jul 10, 2023

View reviewed changes

kirankotari and others added 5 commits July 10, 2023 16:38

Update pyang/plugins/uml.py

a58f297

updating `--uml-add-legend` help Co-authored-by: Nick Hancock <nick.hancock@adtran.com>

Update pyang/plugins/uml.py

ae249d6

updating to module Co-authored-by: Nick Hancock <nick.hancock@adtran.com>

Update pyang/plugins/uml.py

23f5bc4

legend msg update Co-authored-by: Nick Hancock <nick.hancock@adtran.com>

opt reformated

0d27a44

pull out legend

d98c82b

kirankotari changed the title ~~adding --uml-skip-module, --uml-add-legend~~ adding --uml-no-inline-groupings-from Jul 11, 2023

kirankotari added 2 commits July 11, 2023 18:02

mobing options to right place

1e73120

uml options update

1e5e94d

kirankotari requested a review from nkhancock July 11, 2023 22:15

nkhancock reviewed Jul 12, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding --uml-no-inline-groupings-from #872

adding --uml-no-inline-groupings-from #872

kirankotari commented Jul 8, 2023

nkhancock Jul 10, 2023

nkhancock Jul 10, 2023

nkhancock Jul 10, 2023 •

edited

Loading

kirankotari Jul 10, 2023

nkhancock Jul 10, 2023 •

edited

Loading

kirankotari Jul 10, 2023

nkhancock Jul 11, 2023 •

edited

Loading

nkhancock Jul 10, 2023

kirankotari Jul 11, 2023

nkhancock Jul 11, 2023 •

edited

Loading

nkhancock Jul 10, 2023

nkhancock Jul 10, 2023 •

edited

Loading

kirankotari commented Jul 10, 2023

nkhancock Jul 12, 2023

nkhancock Jul 12, 2023

adding --uml-no-inline-groupings-from #872

Are you sure you want to change the base?

adding --uml-no-inline-groupings-from #872

Conversation

kirankotari commented Jul 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkhancock Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkhancock Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkhancock Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkhancock Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkhancock Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

kirankotari commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkhancock Jul 10, 2023 •

edited

Loading

nkhancock Jul 10, 2023 •

edited

Loading

nkhancock Jul 11, 2023 •

edited

Loading

nkhancock Jul 11, 2023 •

edited

Loading

nkhancock Jul 10, 2023 •

edited

Loading