From 1614f7027de6cc2b78cb81991811a8e96422beb9 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 7 Oct 2026 16:32:11 -0400 Subject: [PATCH] fix(dumpkit): tree output raises on complex and is undocumented as lossy (#1266) - make_dumper() overrides Tree._append_number to write a complex value as "-> (1+2j)", spelling NaN/Infinity as the writer does for a float; upstream math.isnan() raised TypeError on it. JSON and PLIST unchanged. - docs/source/pcapkit/dumpkit/index.rst: state that tree is a lossy, human-readable view and that json and plist are the round-trip formats. - tests/dumpkit/test_tree_lossy_unit.py pins both. --- docs/source/pcapkit/dumpkit/index.rst | 8 +++ pcapkit/dumpkit/common.py | 30 ++++++++ tests/dumpkit/test_tree_lossy_unit.py | 99 +++++++++++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 tests/dumpkit/test_tree_lossy_unit.py diff --git a/docs/source/pcapkit/dumpkit/index.rst b/docs/source/pcapkit/dumpkit/index.rst index e8f4126504..7d7904609a 100644 --- a/docs/source/pcapkit/dumpkit/index.rst +++ b/docs/source/pcapkit/dumpkit/index.rst @@ -48,3 +48,11 @@ hierarchy: click Dumper "/pcapkit/dumpkit/common.html#pcapkit.dumpkit.common.Dumper" click PCAPIO "/pcapkit/dumpkit/pcap.html#pcapkit.dumpkit.pcap.PCAPIO" click NotImplementedIO "/pcapkit/dumpkit/null.html#pcapkit.dumpkit.null.NotImplementedIO" + +.. note:: + + The ``tree`` output (also selected as ``text`` and ``txt``) is a lossy, + human-readable view, and nothing reads it back. It renders ``''``, ``b''``, + :data:`None` and ``{}`` all as ``NIL``, and writes a newline inside a string + as is, breaking the layout. Use ``json`` or ``plist`` (also selected as + ``xml``) where the report has to round-trip. diff --git a/pcapkit/dumpkit/common.py b/pcapkit/dumpkit/common.py index 47a6d34762..c0d1d169eb 100644 --- a/pcapkit/dumpkit/common.py +++ b/pcapkit/dumpkit/common.py @@ -25,6 +25,7 @@ import dictdumper.dumper import dictdumper.json import dictdumper.plist +import dictdumper.tree from pcapkit.corekit.infoclass import Info from pcapkit.corekit.multidict import MultiDict, OrderedMultiDict @@ -306,6 +307,12 @@ def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]': # report has (GitHub issue #1257). The defect is :mod:`dictdumper`'s, so it # is worked around here. keep_none = issubclass(output, dictdumper.plist.PLIST) + # NOTE: :meth:`~dictdumper.tree.Tree._append_number` hands every number to + # :func:`math.isnan`, which raises :exc:`TypeError` for a :class:`complex` + # one, although :class:`~dictdumper.tree.Tree` routes :class:`complex` there + # (GitHub issue #1266). The defect is :mod:`dictdumper`'s, so it is worked + # around here. + tree_numbers = issubclass(output, dictdumper.tree.Tree) def escape_key(key: 'Any') -> 'Any': """Escape a mapping key on its way to the writer. @@ -510,6 +517,29 @@ def _append_string(self, value: 'str', file: 'TextIO') -> 'None': text.append(json.dumps(char)[1:-1] if ord(char) > 0xFFFF else f'\\u{ord(char):04x}') file.write('"' + ''.join(text) + '"') + if tree_numbers: + def _append_number(self, value: 'int | float | complex', file: 'TextIO') -> 'None': + """Call this function to write number contents. + + Args: + self: Dumper instance. + value: Content to be dumped. + file: Output file. + + Notes: + A :class:`complex` value is written as ``-> (1+2j)``, with + ``NaN`` and ``Infinity`` spelt in either part as + :meth:`dictdumper.tree.Tree._append_number` spells them for a + :class:`float`; every other number is written by the + upstream writer. + + """ + if not isinstance(value, complex): + return super()._append_number(value, file) + text = str(value).replace('nan', 'NaN').replace('inf', 'Infinity') + file.write(f'-> {text}') + return None + def default(self, o: 'Any') -> 'Literal["fallback"]': # pylint: disable=unused-argument """Check content type for function call. diff --git a/tests/dumpkit/test_tree_lossy_unit.py b/tests/dumpkit/test_tree_lossy_unit.py new file mode 100644 index 0000000000..e9d5a98b4c --- /dev/null +++ b/tests/dumpkit/test_tree_lossy_unit.py @@ -0,0 +1,99 @@ +# -*- coding: utf-8 -*- +"""The ``tree`` dumper writes a :class:`complex` number instead of raising. + +GitHub issue #1266: :meth:`dictdumper.tree.Tree._append_number` hands every +number to :func:`math.isnan`, which raises ``TypeError: must be real number, not +complex`` for the :class:`complex` values :class:`~dictdumper.tree.Tree` routes +there. :func:`~pcapkit.dumpkit.common.make_dumper` now writes those itself, in +the writer's own ``-> `` style. ``json`` and ``plist`` are untouched: +they already write a :class:`complex` as the string ``(1+2j)``. + +:mod:`pcapkit` is imported inside each test, after +:func:`~tests._support.reimport_once_per_class`, not at module load, so that the +dumper is built from the import that is live for this class. + +""" + +import json +import os +import plistlib +import tempfile +import unittest + +from tests._support import reimport_once_per_class + +#: Name passed to every dumper call. +NAME = 'x' + + +class TestTreeComplexNumbers(unittest.TestCase): + """Pin how each writer renders a :class:`complex` value.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.tmp = tmp.name + + def dump(self, writer: str, value: object) -> str: + """Write ``value`` through ``make_dumper(dictdumper.)``.""" + import importlib + + dictdumper = importlib.import_module('dictdumper') + common = importlib.import_module('pcapkit.dumpkit.common') + + path = os.path.join(self.tmp, f'{writer}.out') + dumper = common.make_dumper(getattr(dictdumper, writer))(path) + dumper(value, name=NAME) + with open(path, encoding='utf-8') as file: + return file.read() + + def test_tree_writes_top_level_complex(self) -> None: + self.assertEqual(self.dump('Tree', {'c': 1 + 2j}), + f'{NAME}\n |-- c -> (1+2j)\n') + + def test_tree_writes_nested_complex(self) -> None: + text = self.dump('Tree', {'outer': {'inner': -0.5 - 3j}}) + self.assertIn('|-- inner -> (-0.5-3j)', text) + + def test_tree_writes_complex_in_list(self) -> None: + text = self.dump('Tree', {'items': [1, 2j, 1.5]}) + self.assertIn('-> 2j', text) + self.assertIn('-> 1', text) + self.assertIn('-> 1.5', text) + + def test_tree_spells_non_finite_parts_as_for_float(self) -> None: + text = self.dump('Tree', {'c': complex(float('inf'), float('nan')), + 'f': float('inf')}) + self.assertIn('|-- c -> (Infinity+NaNj)', text) + self.assertIn('|-- f -> Infinity', text) + + def test_tree_writes_real_numbers_as_upstream(self) -> None: + self.assertEqual(self.dump('Tree', {'i': 1, 'f': 1.5, 'n': float('nan')}), + f'{NAME}\n |-- i -> 1\n |-- f -> 1.5\n |-- n -> NaN\n') + + def test_json_and_plist_keep_complex_as_string(self) -> None: + value = {'c': 1 + 2j, 'outer': {'inner': 2j}, 'items': [2j]} + expected = {NAME: {'c': '(1+2j)', 'outer': {'inner': '2j'}, 'items': ['2j']}} + self.assertEqual( + self.dump('JSON', value), + '{\n\t"x": {\n\t\t"c": "(1+2j)",\n\t\t"outer": {\n\t\t\t"inner": "2j"\n' + '\t\t},\n\t\t"items": [ "2j" ]\n\t}\n}', + ) + self.assertEqual(json.loads(self.dump('JSON', value)), expected) + self.assertEqual(plistlib.loads(self.dump('PLIST', value).encode('utf-8')), expected) + + def test_only_tree_overrides_append_number(self) -> None: + import importlib + + dictdumper = importlib.import_module('dictdumper') + common = importlib.import_module('pcapkit.dumpkit.common') + + for writer in ('JSON', 'PLIST'): + with self.subTest(writer=writer): + upstream = getattr(dictdumper, writer) + self.assertNotIn('_append_number', vars(common.make_dumper(upstream))) + + +if __name__ == '__main__': + unittest.main()