Analysis of some mypy adoption problems and the introduction of type: ignore comments

[bizfsc]

Analysis of type: ignore Comments and Potential Fixes

This issue explains why multiple type: ignore comments were introduced
during the mypy adoption (PRs #647, #649, #650, #651) and what API changes
or refactorings would be needed to eliminate them.

Root Cause: Dynamically Typed Value Pipeline

The core issue is that ODVariable.decode_raw() returns
Union[int, float, str, bytes, bytearray] and encode_raw() accepts the same
union. This "wide" union type propagates through the entire Variable property
chain (raw, phys, desc, bits), making it impossible for mypy to know
which concrete type is in play at any given point.

In practice, the concrete type is determined by ODVariable.data_type at
runtime — but mypy cannot track this correlation.

---

Per-File Analysis

canopen/objectdictionary/__init__.py (7 ignores)

1. encode_raw() — string operations on Union type

return value.encode("ascii")   # type: ignore[union-attr]
return value.encode("utf_16_le")  # type: ignore[union-attr]
return bytes(value)            # type: ignore[arg-type]

Why: After the isinstance(value, (bytes, bytearray)) guard, value is
narrowed to Union[int, float, str]. The VISIBLE_STRING / UNICODE_STRING
branches assume value is str, but mypy doesn't correlate data_type with
the value type.

Fix: Use @overload to create typed variants of encode_raw():

@overload
def encode_raw(self, value: str) -> bytes: ...
@overload
def encode_raw(self, value: int) -> bytes: ...
@overload
def encode_raw(self, value: bytes) -> bytes: ...

Or split into encode_string(), encode_number(), encode_bytes().
This is a significant API change.

2. encode_raw() — min/max comparisons

if self.min is not None and value < self.min:  # type: ignore[operator]
if self.max is not None and value > self.max:  # type: ignore[operator]

Why: At this point value is Union[int, float, str], but the comparison
only makes sense for numbers. The str type doesn't support </> with int.

Fix: Add an explicit isinstance(value, (int, float)) guard before the
comparisons. This would be safe and backward-compatible:

if isinstance(value, (int, float)):
    if self.min is not None and value < self.min:
        ...

3. encode_phys() — division and return

value = int(round(value / self.factor))  # type: ignore[operator]
return value  # type: ignore[return-value]

Why: value is Union[int, bool, float, str, bytes] but division only
works on numbers. The return value may still be str or bytes when
data_type is not in INTEGER_TYPES.

Fix: Change encode_phys() to accept and return Union[int, float] only.
Callers that pass str/bytes would need to go through encode_raw() instead.
This is an API change.

4. File handling in export_od() / import_od()

dest.close()     # type: ignore[union-attr]
filename = source.name  # type: ignore[union-attr]

Why: dest is Union[str, TextIO, None] — after the opened_here guard,
it's always TextIO. source after hasattr(source, "read") is TextIO.

Fix: Introduce a local variable with explicit narrowing:

if opened_here:
    assert isinstance(dest, TextIO)
    dest.close()

Or restructure using a context manager pattern. This is a safe, non-breaking
change but requires some refactoring of the function flow.

---

canopen/variable.py (6 ignores)

5. raw property flows into type-specific methods

value = self.od.decode_phys(self.raw)   # type: ignore[arg-type]
value = self.od.decode_desc(self.raw)   # type: ignore[arg-type]
self.raw = self.od.encode_desc(desc)    # type: ignore[assignment]

Why: self.raw returns Union[int, bool, float, str, bytes], but
decode_phys() expects int and decode_desc() expects int.

Fix: This is a consequence of the root cause. Possible approaches:

1. Make decode_phys() / decode_desc() accept the full Union and do
   runtime validation internally.
2. Introduce a Variable.raw_int property that returns int and use it
   in phys and desc properties.
3. Use Generic types parameterized by data_type — very complex.

6. Bits class raw type

return self.variable.od.decode_bits(self.raw, ...)  # type: ignore[arg-type]
self.raw = self.variable.od.encode_bits(self.raw, ...)  # type: ignore[arg-type]

Why: Bits.raw is Union[int, bool, float, str, bytes] (copied from
Variable.raw), but decode_bits() / encode_bits() expect int.

Fix: Bits should store raw as int and cast on read/write:

def read(self):
    raw = self.variable.raw
    assert isinstance(raw, int)
    self.raw: int = raw

7. write() method — desc assignment

self.desc = value  # type: ignore[assignment]

Why: value is Union[int, bool, float, str, bytes] but desc setter
expects str.

Fix: Add type narrowing or runtime validation in write():

elif fmt == "desc":
    if not isinstance(value, str):
        raise TypeError("desc format requires str value")
    self.desc = value

---

canopen/network.py (2 ignores)

8. Node construction with Optional OD

node = RemoteNode(node, object_dictionary)  # type: ignore[arg-type]
node = LocalNode(node, object_dictionary)   # type: ignore[arg-type]

Why: object_dictionary is Union[str, ObjectDictionary, None], but
node constructors don't accept None.

Fix: Make RemoteNode / LocalNode constructors accept None for
object_dictionary and create an empty ObjectDictionary() internally.
Or add proper validation in add_node() / create_node():

if object_dictionary is None:
    raise ValueError("object_dictionary is required when node is an int")

Note: The current code intentionally allows None to flow through — e.g.
when upload_eds fails, the node is still created with None OD. This is
a design decision that would need discussion.

---

canopen/sdo/base.py (1 ignore)

9. SdoArray.__len__() return type

return self[0].raw  # type: ignore[return-value]

Why: self[0].raw is Union[int, bool, float, str, bytes] but
__len__ must return int.

Fix: Cast explicitly:

def __len__(self) -> int:
    value = self[0].raw
    assert isinstance(value, int)
    return value

---

Summary of Required Changes

Priority	Change	Impact	Breaking?
Low	encode_raw() min/max guards	2 ignores removed	No
Low	Bits.raw as int	2 ignores removed	No
Low	write() desc validation	1 ignore removed	No
Low	SdoArray.__len__ cast	1 ignore removed	No
Low	File handling narrowing	2 ignores removed	No
Medium	decode_phys/decode_desc accept Union	3 ignores removed	Minor
Medium	Node constructor accept None	2 ignores removed	Minor
High	encode_raw overloads or split	3 ignores removed	Yes
High	encode_phys type narrowing	2 ignores removed	Yes

The low priority changes (10 ignores) can be done without any API changes.
The medium/high priority changes (8 ignores) require API discussion and
potentially affect downstream users.