René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Analysis of some mypy adoption problems and the introduction of type: ignore comments","articleBody":"# Analysis of `type: ignore` Comments and Potential Fixes\n\nThis issue explains why multiple `type: ignore` comments were introduced\nduring the mypy adoption (PRs #647, #649, #650, #651) and what API changes\nor refactorings would be needed to eliminate them.\n\n## Root Cause: Dynamically Typed Value Pipeline\n\nThe core issue is that `ODVariable.decode_raw()` returns\n`Union[int, float, str, bytes, bytearray]` and `encode_raw()` accepts the same\nunion. This \"wide\" union type propagates through the entire `Variable` property\nchain (`raw`, `phys`, `desc`, `bits`), making it impossible for mypy to know\nwhich concrete type is in play at any given point.\n\nIn practice, the concrete type is determined by `ODVariable.data_type` at\nruntime — but mypy cannot track this correlation.\n\n---\n\n## Per-File Analysis\n\n### `canopen/objectdictionary/__init__.py` (7 ignores)\n\n#### 1. `encode_raw()` — string operations on Union type\n```python\nreturn value.encode(\"ascii\")   # type: ignore[union-attr]\nreturn value.encode(\"utf_16_le\")  # type: ignore[union-attr]\nreturn bytes(value)            # type: ignore[arg-type]\n```\n**Why:** After the `isinstance(value, (bytes, bytearray))` guard, `value` is\nnarrowed to `Union[int, float, str]`. The `VISIBLE_STRING` / `UNICODE_STRING`\nbranches assume `value` is `str`, but mypy doesn't correlate `data_type` with\nthe value type.\n\n**Fix:** Use `@overload` to create typed variants of `encode_raw()`:\n```python\n@overload\ndef encode_raw(self, value: str) -\u003e bytes: ...\n@overload\ndef encode_raw(self, value: int) -\u003e bytes: ...\n@overload\ndef encode_raw(self, value: bytes) -\u003e bytes: ...\n```\nOr split into `encode_string()`, `encode_number()`, `encode_bytes()`.\nThis is a significant API change.\n\n#### 2. `encode_raw()` — min/max comparisons\n```python\nif self.min is not None and value \u003c self.min:  # type: ignore[operator]\nif self.max is not None and value \u003e self.max:  # type: ignore[operator]\n```\n**Why:** At this point `value` is `Union[int, float, str]`, but the comparison\nonly makes sense for numbers. The `str` type doesn't support `\u003c`/`\u003e` with `int`.\n\n**Fix:** Add an explicit `isinstance(value, (int, float))` guard before the\ncomparisons. This would be safe and backward-compatible:\n```python\nif isinstance(value, (int, float)):\n    if self.min is not None and value \u003c self.min:\n        ...\n```\n\n#### 3. `encode_phys()` — division and return\n```python\nvalue = int(round(value / self.factor))  # type: ignore[operator]\nreturn value  # type: ignore[return-value]\n```\n**Why:** `value` is `Union[int, bool, float, str, bytes]` but division only\nworks on numbers. The `return value` may still be `str` or `bytes` when\n`data_type` is not in `INTEGER_TYPES`.\n\n**Fix:** Change `encode_phys()` to accept and return `Union[int, float]` only.\nCallers that pass `str`/`bytes` would need to go through `encode_raw()` instead.\nThis is an API change.\n\n#### 4. File handling in `export_od()` / `import_od()`\n```python\ndest.close()     # type: ignore[union-attr]\nfilename = source.name  # type: ignore[union-attr]\n```\n**Why:** `dest` is `Union[str, TextIO, None]` — after the `opened_here` guard,\nit's always `TextIO`. `source` after `hasattr(source, \"read\")` is `TextIO`.\n\n**Fix:** Introduce a local variable with explicit narrowing:\n```python\nif opened_here:\n    assert isinstance(dest, TextIO)\n    dest.close()\n```\nOr restructure using a context manager pattern. This is a safe, non-breaking\nchange but requires some refactoring of the function flow.\n\n---\n\n### `canopen/variable.py` (6 ignores)\n\n#### 5. `raw` property flows into type-specific methods\n```python\nvalue = self.od.decode_phys(self.raw)   # type: ignore[arg-type]\nvalue = self.od.decode_desc(self.raw)   # type: ignore[arg-type]\nself.raw = self.od.encode_desc(desc)    # type: ignore[assignment]\n```\n**Why:** `self.raw` returns `Union[int, bool, float, str, bytes]`, but\n`decode_phys()` expects `int` and `decode_desc()` expects `int`.\n\n**Fix:** This is a consequence of the root cause. Possible approaches:\n1. Make `decode_phys()` / `decode_desc()` accept the full Union and do\n   runtime validation internally.\n2. Introduce a `Variable.raw_int` property that returns `int` and use it\n   in `phys` and `desc` properties.\n3. Use `Generic` types parameterized by `data_type` — very complex.\n\n#### 6. `Bits` class raw type\n```python\nreturn self.variable.od.decode_bits(self.raw, ...)  # type: ignore[arg-type]\nself.raw = self.variable.od.encode_bits(self.raw, ...)  # type: ignore[arg-type]\n```\n**Why:** `Bits.raw` is `Union[int, bool, float, str, bytes]` (copied from\n`Variable.raw`), but `decode_bits()` / `encode_bits()` expect `int`.\n\n**Fix:** `Bits` should store `raw` as `int` and cast on read/write:\n```python\ndef read(self):\n    raw = self.variable.raw\n    assert isinstance(raw, int)\n    self.raw: int = raw\n```\n\n#### 7. `write()` method — desc assignment\n```python\nself.desc = value  # type: ignore[assignment]\n```\n**Why:** `value` is `Union[int, bool, float, str, bytes]` but `desc` setter\nexpects `str`.\n\n**Fix:** Add type narrowing or runtime validation in `write()`:\n```python\nelif fmt == \"desc\":\n    if not isinstance(value, str):\n        raise TypeError(\"desc format requires str value\")\n    self.desc = value\n```\n\n---\n\n### `canopen/network.py` (2 ignores)\n\n#### 8. Node construction with Optional OD\n```python\nnode = RemoteNode(node, object_dictionary)  # type: ignore[arg-type]\nnode = LocalNode(node, object_dictionary)   # type: ignore[arg-type]\n```\n**Why:** `object_dictionary` is `Union[str, ObjectDictionary, None]`, but\nnode constructors don't accept `None`.\n\n**Fix:** Make `RemoteNode` / `LocalNode` constructors accept `None` for\n`object_dictionary` and create an empty `ObjectDictionary()` internally.\nOr add proper validation in `add_node()` / `create_node()`:\n```python\nif object_dictionary is None:\n    raise ValueError(\"object_dictionary is required when node is an int\")\n```\nNote: The current code intentionally allows `None` to flow through — e.g.\nwhen `upload_eds` fails, the node is still created with `None` OD. This is\na design decision that would need discussion.\n\n---\n\n### `canopen/sdo/base.py` (1 ignore)\n\n#### 9. `SdoArray.__len__()` return type\n```python\nreturn self[0].raw  # type: ignore[return-value]\n```\n**Why:** `self[0].raw` is `Union[int, bool, float, str, bytes]` but\n`__len__` must return `int`.\n\n**Fix:** Cast explicitly:\n```python\ndef __len__(self) -\u003e int:\n    value = self[0].raw\n    assert isinstance(value, int)\n    return value\n```\n\n---\n\n## Summary of Required Changes\n\n| Priority | Change | Impact | Breaking? |\n|----------|--------|--------|-----------|\n| Low | `encode_raw()` min/max guards | 2 ignores removed | No |\n| Low | `Bits.raw` as `int` | 2 ignores removed | No |\n| Low | `write()` desc validation | 1 ignore removed | No |\n| Low | `SdoArray.__len__` cast | 1 ignore removed | No |\n| Low | File handling narrowing | 2 ignores removed | No |\n| Medium | `decode_phys`/`decode_desc` accept Union | 3 ignores removed | Minor |\n| Medium | Node constructor accept None | 2 ignores removed | Minor |\n| High | `encode_raw` overloads or split | 3 ignores removed | Yes |\n| High | `encode_phys` type narrowing | 2 ignores removed | Yes |\n\nThe **low priority** changes (10 ignores) can be done without any API changes.\nThe **medium/high priority** changes (8 ignores) require API discussion and\npotentially affect downstream users.\n","author":{"url":"https://github.com/bizfsc","@type":"Person","name":"bizfsc"},"datePublished":"2026-04-29T19:25:22.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":0},"url":"https://github.com/652/canopen/issues/652"}