mercedes-benz / odxtools

odxtools is a collection of utilities to interact with the diagnostic functionality of automotive electronic control units using python
MIT License
171 stars 70 forks source link

tablekeyparameter: fix short name reference resolution #325

Closed andlaus closed 1 month ago

andlaus commented 1 month ago

unfortunately, the Table and TableRow classes are only available during type checking (i.e., not during normal execution) because this would lead to circular imports.

this should fix https://github.com/mercedes-benz/odxtools/issues/324.

Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH. Provider Information

kayoub5 commented 1 month ago

Not possible to just remove if TYPE_CHECKING: in line 17 ?

andlaus commented 1 month ago

unfortunately not. this leads to cylic imports:

> pytest --tb=native .
ERROR: while parsing the following warning configuration:

  error::odxtools.exceptions.DecodeError

This error occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/_pytest/config/__init__.py", line 1921, in parse_warning_filter
    category: Type[Warning] = _resolve_warning_category(category_)
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/_pytest/config/__init__.py", line 1959, in _resolve_warning_category
    m = __import__(module, None, None, [klass])
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/and/src/odxtools/odxtools/__init__.py", line 65, in <module>
    from .loadfile import load_directory as load_directory  # noqa: F401
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/and/src/odxtools/odxtools/loadfile.py", line 6, in <module>
    from .database import Database
  File "/home/and/src/odxtools/odxtools/database.py", line 12, in <module>
    from .diaglayercontainer import DiagLayerContainer
  File "/home/and/src/odxtools/odxtools/diaglayercontainer.py", line 9, in <module>
    from .diaglayers.basevariant import BaseVariant
  File "/home/and/src/odxtools/odxtools/diaglayers/basevariant.py", line 9, in <module>
    from ..diagvariable import DiagVariable
  File "/home/and/src/odxtools/odxtools/diagvariable.py", line 7, in <module>
    from .commrelation import CommRelation
  File "/home/and/src/odxtools/odxtools/commrelation.py", line 10, in <module>
    from .diagservice import DiagService
  File "/home/and/src/odxtools/odxtools/diagservice.py", line 15, in <module>
    from .request import Request
  File "/home/and/src/odxtools/odxtools/request.py", line 6, in <module>
    from .basicstructure import BasicStructure
  File "/home/and/src/odxtools/odxtools/basicstructure.py", line 18, in <module>
    from .parameters.createanyparameter import create_any_parameter_from_et
  File "/home/and/src/odxtools/odxtools/parameters/createanyparameter.py", line 18, in <module>
    from .tablekeyparameter import TableKeyParameter
  File "/home/and/src/odxtools/odxtools/parameters/tablekeyparameter.py", line 17, in <module>
    from ..table import Table
  File "/home/and/src/odxtools/odxtools/table.py", line 14, in <module>
    from .tablerow import TableRow
  File "/home/and/src/odxtools/odxtools/tablerow.py", line 6, in <module>
    from .basicstructure import BasicStructure
ImportError: cannot import name 'BasicStructure' from partially initialized module 'odxtools.basicstructure' (most likely due to a circular import) (/home/and/src/odxtools/odxtools/basicstructure.py)
andlaus commented 1 month ago

@lz4v4: have you tested this?

kayoub5 commented 1 month ago

Do the import inside _resolve_snrefs, that way it's lazy and does not cause circular reference

andlaus commented 1 month ago

Do the import inside _resolve_snrefs, that way it's lazy and does not cause circular reference

I'd prefer not to since I'm unsure about the performance implications. (and IMO it is very inelegant as well...)

anyway, since @lz4v4 confirmed that the patch works, let's merge it for now and maybe switch to another solution later.