Reduced the type: ignore comments and allocated the structured dataclass #393

vinothk-master · 2025-02-22T17:29:03Z

Added these set of lines which is used to reduce the type :ignore comments in compare.py file which increases the readability and to maintain a structured dict. Please check these line commits and let me know I can help on this regard.

class SpecsServiceDict(TypedDict): diag_layer: str diag_layer_type: str new_services: List[str] # List of new service names deleted_services: List[DiagService] # List of deleted services renamed_service: List[Tuple[str, DiagService]] # List of (old_name, DiagService) changed_name_of_service: List[Tuple[str, DiagService]] # Similar to renamed_service changed_parameters_of_service: List[Tuple[str, DiagService, List[Union[str, int, float]]]] class SpecsChangesVariants(TypedDict): new_variants : List[str] deleted_variants : List[str] service_changes : SpecsServiceDict

Thanks
Vinoth Kannan

CLA-Mercedes-Benz · 2025-02-22T17:29:09Z

All committers have signed the CLA.

odxtools/cli/compare.py

andlaus

In principle, I like it. IMO -- besides the usual bits and pieces that need to be fixed -- for whatever reason it seems that a lot of unrelated formatting changes slipped into this (which makes reviewing it more difficult than it needs to be and also prevents the CI from passing). To fix the formatting, install yapf and isort via pip and run the reformat-source.sh script in the toplevel directory of your working copy...

odxtools/cli/compare.py

vinothk-master · 2025-02-24T21:15:08Z

Member

Overall executed the reformat-source.sh as a result it would satisfy the formatting conditions.

$ sh reformat-source.sh
Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\basicstructure.py
Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\nameditemlist.py
Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\odxtypes.py
Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\request.py
Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\response.py
Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\cli\compare.py
Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\diaglayers\hierarchyelement.py
Fixing E:\OpenSource Contributions\Mercedes\odxtools\tests\test_compu_methods.py

…rmat-source.sh etc.

andlaus · 2025-02-25T10:59:41Z

Overall executed the reformat-source.sh as a result it would satisfy the formatting conditions.

yes. ideally you run this script before you commit anything, so you don't need to worry about the formatting...

andlaus · 2025-02-25T11:01:38Z

(before you push to github, it is IMO advisable to run mypy and ruff as well because these are build into the CI and your PR will fail if these two do not pass locally...)

vinothk-master · 2025-02-25T11:05:36Z

(before you push to github, it is IMO advisable to run mypy and ruff as well because these are build into the CI and your PR will fail if these two do not pass locally...)

Ok let me work on it and thanks for letting me know

…o refractor_compare_tool

…sh file

…sh fil

…o refractor_compare_tool

vinothk-master

1. refractored the code by adding dataclass and formated using reformat.sh
2. Verfied using ruff and mypy

odxtools/cli/.gitignore

odxtools/cli/compare.py

kayoub5 · 2025-03-23T23:43:22Z

odxtools/cli/compare.py

+class ServiceSpecs:
+    diag_layer: str
+    diag_layer_type: str
+    new_services: List[DiagService]  # List of new service name


comments that just repeat variable name are redundant

odxtools/cli/compare.py

andlaus

are you sure that you have run the reformatting script? On my machine it still conducts quite a few changes on this PR:

~/src/odxtools|7f75ca4... > ./reformat-source.sh
Fixing /home/and/src/odxtools/odxtools/request.py
Fixing /home/and/src/odxtools/odxtools/response.py
Fixing /home/and/src/odxtools/odxtools/odxtypes.py
Fixing /home/and/src/odxtools/odxtools/version.py
Fixing /home/and/src/odxtools/odxtools/nameditemlist.py
Fixing /home/and/src/odxtools/odxtools/basicstructure.py
Fixing /home/and/src/odxtools/odxtools/cli/compare.py
Fixing /home/and/src/odxtools/odxtools/diaglayers/hierarchyelement.py
Fixing /home/and/src/odxtools/tests/test_compu_methods.py
~/src/odxtools|7f75ca4... > git diff --stat
 odxtools/cli/compare.py | 398 +++++++++++++++++-------------------------------
 1 file changed, 136 insertions(+), 262 deletions(-)

odxtools/cli/compare.py

andlaus · 2025-03-24T17:02:08Z

odxtools/cli/compare.py


-        if service_spec["deleted_services"]:
+        if service_specs.deleted_services:
+            assert isinstance(service_specs.deleted_services, List)


that assert should be superfluous now...

andlaus · 2025-03-24T17:04:00Z

odxtools/cli/compare.py

-        if service_spec["changed_name_of_service"][0]:
+            rich_print(extract_service_tabulation_data(service_specs.deleted_services))
+        if service_specs.changed_name_of_service:
+            # assert isinstance(service_specs.changed_name_of_service[0], List)


only leave such "ruins" in the code base in exceptional circumstances (IMO this is not one of these)

andlaus · 2025-03-24T17:07:06Z

odxtools/cli/compare.py

-            for variant in changes_variants["deleted_diagnostic_layers"]:
-                assert isinstance(variant, DiagLayer)
-                rich_print(
-                    f"  [magenta]{variant.short_name}[/magenta] ({variant.variant_type.value})")
-
-        # diagnostic services
-        for _, value in changes_variants.items():
-            if isinstance(value, dict):
-                self.print_dl_changes(value)


where did this go?

odxtools/cli/compare.py

andlaus · 2025-03-24T17:12:26Z

odxtools/cli/compare.py

                    # add old service name (type: String)
-                    service_dict["changed_name_of_service"][1].append(service2.short_name)
+                    #                    service_specs.changed_name_of_service[1].append(service2.short_name)


this seems to have gone MIA?

odxtools/cli/compare.py

vinothk-master · 2025-03-27T23:49:09Z

executed the reformat-source.sh
$ sh reformat-source.sh Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\basicstructure.py Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\nameditemlist.py Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\odxtypes.py Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\request.py Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\response.py Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\cli\compare.py Fixing E:\OpenSource Contributions\Mercedes\odxtools\odxtools\diaglayers\hierarchyelement.py Fixing E:\OpenSource Contributions\Mercedes\odxtools\tests\test_compu_methods.py Skipped 1 files
Executed mypy and Ruff

andlaus · 2025-04-16T16:31:24Z

since we raised the minimum python version to 3.10, ruff now complains. the good news is that running ruff check --fix will take care of most -- maybe all -- issues. besides that, the PR can be merged...

vinothk-master · 2025-04-16T16:35:02Z

since we raised the minimum python version to 3.10, ruff now complains. the good news is that running ruff check --fix will take care of most -- maybe all -- issues. besides that, the PR can be merged...

@andlaus I used the same command ruff check --fix . as you have mentioned before, but still it is failing!

vinothk-master · 2025-04-16T16:36:14Z

since we raised the minimum python version to 3.10, ruff now complains. the good news is that running ruff check --fix will take care of most -- maybe all -- issues. besides that, the PR can be merged...

@andlaus I used the same command ruff check --fix . as you have mentioned before, but still it is failing!
d $ODXTOOLS_SOURCE_DIR git checkout main git pull git checkout -b WIP git checkout refractor_compare_tool odxtools/cli/compare.py ruff check --fix . ./reformat-source.sh git commit -a -m "refactor the compare tool to reduce the number of type: ignore comments" git checkout refractor_compare_tool git reset --hard WIP git push -f origin refractor_compare_tool
Is there anything else we have do inorder to proceed with this linting issue?

Thanks

vinothk-master · 2025-04-17T13:23:45Z

Hi @andlaus

Can you please help me with resolving this lint issue. I executed the same ruff command in local repo but after resolving the conflicts it is failing I guess.

Thanks
Vinoth Kannan

andlaus · 2025-04-20T07:54:15Z

Can you please help me with resolving this lint issue

weird: it seems like that for whatever reason, ruff starts complaining about the auto-generated file version.py. this can be fixed by adding "odxtools/version.py", to tool.ruff.exclude in pyproject.toml.

….toml

andlaus · 2025-04-20T16:42:56Z

thanks for the fix. it still does not work, but I suspect that this is due to leftover artifacts from packaging (the offending file is in the build/ sub directory). I'll fix this when I'm back from easter (i.e., Tuesday)

for whatever reason, ruff starts to care about the `build/` subdirectory with mercedes-benz#393 . Since this is probably an artifact of pip, let's just delete this directory before linting. Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com> Signed-off-by: Christian Hackenbeck <christian.hackenbeck@mercedes-benz.com>

andlaus · 2025-04-22T07:45:04Z

I opened a PR for this PR: vinothk-master#1 . I hope that after you've merged it, the issue is fixed. Note that, due to my unfamiliarity with github actions, I was unable to test if it works...

for whatever reason, ruff starts to care about the `build/` subdirectory with mercedes-benz#393 . Since this is probably an artifact of pip, let's just delete this directory before linting. Signed-off-by: Andreas Lauser <andreas.lauser@mercedes-benz.com> Signed-off-by: Christian Hackenbeck <christian.hackenbeck@mercedes-benz.com>

attempt to fix the github actions

vinothk-master · 2025-04-22T20:23:23Z

I opened a PR for this PR: vinothk-master#1 . I hope that after you've merged it, the issue is fixed. Note that, due to my unfamiliarity with github actions, I was unable to test if it works...

Thanks @andlaus ! I have merged your workflow fix.

.gitignore

odxtools/cli/compare.py

…o refractor_compare_tool

…aster/odxtools into refractor_compare_tool

odxtools/cli/compare.py

andlaus · 2025-04-23T18:20:50Z

seems like my fix worked. merging, thanks for your patience!

vinothk-master · 2025-04-23T19:23:32Z

seems like my fix worked. merging, thanks for your patience!

Thanks @andlaus and @kayoub5 for helping me in merging this PR

vinothk-master added 2 commits February 22, 2025 17:19

Reduced the type: ignore comments and allocated the structured dataclass

86b1ae5

Reduced the type: ignore comments and allocated the structured dataclass

38dec1f

kayoub5 reviewed Feb 22, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

kayoub5 reviewed Feb 22, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

kayoub5 reviewed Feb 22, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

fixed the lint issues

824a499

andlaus reviewed Feb 24, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

odxtools/cli/compare.py Outdated Show resolved Hide resolved

odxtools/cli/compare.py Outdated Show resolved Hide resolved

odxtools/cli/compare.py Outdated Show resolved Hide resolved

fixed the issues : change the name to service_spec, executed the refo…

716acc5

…rmat-source.sh etc.

vinothk-master and others added 9 commits March 22, 2025 12:45

Merge branch 'main' of https://github.com/vinothk-master/odxtools int…

89f91f2

…o refractor_compare_tool

refractored the code by adding dataclass and formated using reformat.sh

22241df

Merge branch 'mercedes-benz:main' into refractor_compare_tool

4da9b80

refractored the code by adding dataclass and formated using reformat.…

c6e45d7

…sh file

refractored the code by adding dataclass and formated using reformat.…

cf1288b

…sh file

Merge branch 'refractor_compare_tool'

87de2a2

refractored the code by adding dataclass and formated using reformat.…

c32a2cf

…sh fil

refractored the code by adding dataclass and formated using reformat.sh

4d35bd9

Merge branch 'main' of https://github.com/vinothk-master/odxtools int…

7f75ca4

…o refractor_compare_tool

vinothk-master commented Mar 23, 2025

View reviewed changes

kayoub5 requested changes Mar 23, 2025

View reviewed changes

andlaus reviewed Mar 24, 2025

View reviewed changes

vinothk-master added 4 commits March 27, 2025 23:51

Refractoring the code base with dataclass

e7bfc30

adding to remote branch

161bf41

Remove odxtools/cli/.gitignore from the repository

ef7660e

Removing the obselete codes

9f1c890

vinothk-master force-pushed the refractor_compare_tool branch from c82509d to 23f1323 Compare April 16, 2025 12:17

Merge branch 'main' into refractor_compare_tool

8c49fba

fixed by adding odxtools/version.py to tool.ruff.exclude in pyproject…

5455628

….toml

andlaus mentioned this pull request Apr 22, 2025

attempt to fix the github actions vinothk-master/odxtools#1

Merged

andlaus and others added 2 commits April 22, 2025 09:46

Merge pull request #1 from andlaus/PR393_fix

1595b91

attempt to fix the github actions

kayoub5 reviewed Apr 22, 2025

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

kayoub5 reviewed Apr 22, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

vinothk-master added 4 commits April 23, 2025 14:16

Merge branch 'main' of https://github.com/vinothk-master/odxtools int…

c59c54a

…o refractor_compare_tool

Restore .gitignore from upstream

bb7482c

Merge branch 'refractor_compare_tool' of https://github.com/vinothk-m…

dc49f3b

…aster/odxtools into refractor_compare_tool

Restored .gitignore from upstream and modified the type error

1f17610

kayoub5 reviewed Apr 23, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

kayoub5 reviewed Apr 23, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

kayoub5 reviewed Apr 23, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

kayoub5 reviewed Apr 23, 2025

View reviewed changes

odxtools/cli/compare.py Outdated Show resolved Hide resolved

Reverting all the typo from List to list

45172d9

andlaus merged commit a602a79 into mercedes-benz:main Apr 23, 2025
7 checks passed

andlaus mentioned this pull request Apr 25, 2025

refactor the ´compare´ tool #363

Open

Reduced the type: ignore comments and allocated the structured dataclass #393

Reduced the type: ignore comments and allocated the structured dataclass #393

Uh oh!

Conversation

vinothk-master commented Feb 22, 2025

Uh oh!

CLA-Mercedes-Benz commented Feb 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andlaus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vinothk-master commented Feb 24, 2025

Uh oh!

andlaus commented Feb 25, 2025

Uh oh!

andlaus commented Feb 25, 2025

Uh oh!

vinothk-master commented Feb 25, 2025

Uh oh!

vinothk-master left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kayoub5 Mar 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andlaus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andlaus Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

andlaus Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

andlaus Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andlaus Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vinothk-master commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andlaus commented Apr 16, 2025

Uh oh!

vinothk-master commented Apr 16, 2025

Uh oh!

vinothk-master commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vinothk-master commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andlaus commented Apr 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andlaus commented Apr 20, 2025

Uh oh!

andlaus commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vinothk-master commented Apr 22, 2025

CLA-Mercedes-Benz commented Feb 22, 2025 •

edited

Loading

vinothk-master commented Mar 27, 2025 •

edited

Loading

vinothk-master commented Apr 16, 2025 •

edited

Loading

vinothk-master commented Apr 17, 2025 •

edited

Loading

andlaus commented Apr 20, 2025 •

edited

Loading

andlaus commented Apr 22, 2025 •

edited

Loading