Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change list command output from tabulate to rich tables #359

Merged
merged 12 commits into from
Nov 20, 2024
Merged

change list command output from tabulate to rich tables #359

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bf581dd
change list command output from tabulate to rich tables
Haris-Ali007 Nov 3, 2024
ee1e7ec
Add indentation to rich tables
Haris-Ali007 Nov 4, 2024
129262f
fix coding style errors
Haris-Ali007 Nov 13, 2024
6f9f2a8
Merge branch 'mercedes-benz:main' into main
Haris-Ali007 Nov 13, 2024
1b69202
Resolve static type checking error in _print_utils.py
Haris-Ali007 Nov 13, 2024
01dc11d
Merge branch 'main' of https://github.com/Haris-Ali007/odxtools
Haris-Ali007 Nov 13, 2024
d500a00
Update browse.py from tabulate to rich table
Haris-Ali007 Nov 15, 2024
6be65e1
Remove commented code
Haris-Ali007 Nov 15, 2024
48fa95d
Fix lint coding style issues
Haris-Ali007 Nov 15, 2024
4797cfb
Fix lint code quality issue
Haris-Ali007 Nov 15, 2024
43261c8
Fix static type definition for paramters
Haris-Ali007 Nov 19, 2024
cf6986c
Fix static type definitions for variables defined as List[Any]
Haris-Ali007 Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 58 additions & 45 deletions odxtools/cli/_print_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
from typing import Any, Callable, Dict, List, Optional, Union

import markdownify
from tabulate import tabulate # TODO: switch to rich tables
kayoub5 marked this conversation as resolved.
Show resolved Hide resolved
from rich.table import Table
from rich.padding import Padding

from ..description import Description
from ..diaglayers.diaglayer import DiagLayer
Expand Down Expand Up @@ -82,9 +83,7 @@ def print_service_parameters(service: DiagService,
f" Identifying Prefix: 0x{const_prefix.hex().upper()} ({bytes(const_prefix)!r})")
print_fn(f" Parameters:")
table = extract_parameter_tabulation_data(list(service.request.parameters))
table_str = textwrap.indent(tabulate(table, headers='keys', tablefmt='presto'), " ")
print_fn()
print_fn(table_str)
print_fn(Padding(table, pad=(0, 0, 0, 4)))
print_fn()
else:
print_fn(f" No Request!")
Expand All @@ -97,8 +96,7 @@ def print_service_parameters(service: DiagService,
print_fn(f" Positive Response '{resp.short_name}':")
print_fn(f" Parameters:\n")
table = extract_parameter_tabulation_data(list(resp.parameters))
table_str = textwrap.indent(tabulate(table, headers='keys', tablefmt='presto'), " ")
print_fn(table_str)
print_fn(Padding(table, pad=(0, 0, 0, 4)))
print_fn()

# Negative Response
Expand All @@ -109,8 +107,7 @@ def print_service_parameters(service: DiagService,
print_fn(f" Negative Response '{resp.short_name}':")
print_fn(f" Parameters:\n")
table = extract_parameter_tabulation_data(list(resp.parameters))
table_str = textwrap.indent(tabulate(table, headers='keys', tablefmt='presto'), " ")
kayoub5 marked this conversation as resolved.
Show resolved Hide resolved
print_fn(table_str)
print_fn(Padding(table, pad=(0, 0, 0, 4)))
print_fn()

print_fn("\n")
Expand Down Expand Up @@ -138,15 +135,30 @@ def extract_service_tabulation_data(services: List[DiagService]) -> Dict[str, An
return {'Name': name, 'Semantic': semantic, 'Hex-Request': request}


def extract_parameter_tabulation_data(parameters: List[Parameter]) -> Dict[str, Any]:
def extract_parameter_tabulation_data(parameters: List[Parameter]) -> Table:
# extracts data of parameters of diagnostic services into Dictionary which can be printed by tabulate module
# TODO: consider indentation

name = []
byte = []
bit_length: List[Optional[int]] = []
semantic = []
param_type = []
# Create Rich table
table = Table(
title="", show_header=True, header_style="bold cyan", border_style="blue", show_lines=True)

# Add columns with appropriate styling
table.add_column("Name", style="green")
table.add_column("Byte Position", justify="right", style="yellow")
table.add_column("Bit Length", justify="right", style="yellow")
table.add_column("Semantic", justify="left", style="white")
table.add_column("Parameter Type", justify="left", style="white")
table.add_column("Data Type", justify="left", style="white")
table.add_column("Value", justify="left", style="yellow")
table.add_column("Value Type", justify="left", style="white")
table.add_column("Linked DOP", justify="left", style="white")

name: List[str] = []
byte: List[Any] = []
bit_length: List[Any] = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

that should probably be

Suggested change
byte: List[Any] = []
bit_length: List[Any] = []
byte: List[Optional[int]] = []
bit_length: List[Optional[int]] = []

?

semantic: List[Optional[str]] = []
param_type: List[Optional[str]] = []
value: List[Optional[str]] = []
value_type: List[Optional[str]] = []
data_type: List[Optional[str]] = []
Expand Down Expand Up @@ -216,42 +228,43 @@ def extract_parameter_tabulation_data(parameters: List[Parameter]) -> Dict[str,
value_type.append(None)
dop.append(None)

return {
'Name': name,
'Byte Position': byte,
'Bit Length': bit_length,
'Semantic': semantic,
'Parameter Type': param_type,
'Data Type': data_type,
'Value': value,
'Value Type': value_type,
'Linked DOP': dop
}
for lst in [byte, semantic, bit_length, value, value_type, data_type, dop]:
lst[:] = ["" if x is None else x for x in lst]
Copy link
Collaborator

Choose a reason for hiding this comment

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

great that you made this code here more readably. line 232 should probably better be

Suggested change
lst[:] = ["" if x is None else x for x in lst]
# replace `None` entries by empty strings
lst = ["" if x is None else x for x in lst]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried using simple lst it doesn't override the list in-place.

Copy link
Collaborator

Choose a reason for hiding this comment

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

why does it need to be overwritten in-place? (memory wise it should not make any difference because the current solution creates a temporary list as well and also the required memory hopefully is miniscule...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it does create a temporary list in both case, but in case of list[:] it copies the temporary list to the original address. So in a way I am able to edit the list in it's place.

Copy link
Collaborator

@andlaus andlaus Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

okay, I had a hands-on look and I now understand why the in-place copy of the list is necessary. That said, declaring List[Any] for any of the variables above causes mypy to give up with these variables altogether, so -- while not optimal either -- I prefer to use List[Optional[int]] above and add a # type: ignore[attr-defined, index] comment here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay thanks for the that. I am new to this static type checks. I'll push the changes

# Add all rows at once by zipping dictionary values
rows = zip(name, byte, bit_length, semantic, param_type, data_type, value, value_type, dop)
for row in rows:
table.add_row(*map(str, row))

return table

def print_dl_metrics(variants: List[DiagLayer], print_fn: Callable[..., Any] = print) -> None:

name = []
type = []
num_services = []
num_dops = []
num_comparam_refs = []
def print_dl_metrics(variants: List[DiagLayer], print_fn: Callable[..., Any] = print) -> None:
"""
Print diagnostic layer metrics using Rich tables.
Args:
variants: List of diagnostic layer variants to analyze
print_fn: Optional callable for custom print handling (defaults to built-in print)
"""
# Create Rich table
table = Table(
title="", show_header=True, header_style="bold cyan", border_style="blue", show_lines=True)

# Add columns with appropriate styling
table.add_column("Name", style="green")
table.add_column("Variant Type", style="magenta")
table.add_column("Number of Services", justify="right", style="yellow")
table.add_column("Number of DOPs", justify="right", style="yellow")
table.add_column("Number of communication parameters", justify="right", style="yellow")

# Process each variant
for variant in variants:
assert isinstance(variant, DiagLayer)
all_services: List[Union[DiagService, SingleEcuJob]] = sorted(
variant.services, key=lambda x: x.short_name)
name.append(variant.short_name)
type.append(variant.variant_type.value)
num_services.append(len(all_services))
ddds = variant.diag_data_dictionary_spec
num_dops.append(len(ddds.data_object_props))
num_comparam_refs.append(len(getattr(variant, "comparams_refs", [])))

table = {
'Name': name,
'Variant Type': type,
'Number of Services': num_services,
'Number of DOPs': num_dops,
'Number of communication parameters': num_comparam_refs
}
print_fn(tabulate(table, headers='keys', tablefmt='presto'))

# Add row to table
table.add_row(variant.short_name, variant.variant_type.value, str(len(all_services)),
str(len(ddds.data_object_props)),
str(len(getattr(variant, "comparams_refs", []))))
print_fn(table)
4 changes: 1 addition & 3 deletions odxtools/cli/browse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from typing import List, Optional, Union, cast

import InquirerPy.prompt as IP_prompt
from tabulate import tabulate # TODO: switch to rich tables

from ..database import Database
from ..dataobjectproperty import DataObjectProperty
Expand Down Expand Up @@ -356,8 +355,7 @@ def browse(odxdb: Database) -> None:
if codec is not None:
assert isinstance(codec, (Request, Response))
table = extract_parameter_tabulation_data(codec.parameters)
table_str = tabulate(table, headers='keys', tablefmt='presto')
print(table_str)
print(table)

encode_message_interactively(codec, ask_user_confirmation=True)

Expand Down
Loading