Skip to content

Added support for Tuple data type encoding #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VinciGit00 merged 4 commits into from
Feb 6, 2026
Merged

Added support for Tuple data type encoding #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e510f5
Added encoding support for tuples, they will be handled like lists in…
llakenbr Jan 4, 2026
be0a54f
Test of tuple type
llakenbr Jan 4, 2026
a9f5d65
Test of tuple type
llakenbr Jan 11, 2026
bedbd12
Change of other type description
llakenbr Jan 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions tests/test_encoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ def test_encode_primitive_array():
result = encode(data)
assert result == 'items: [hello,"world, test",foo]'

def test_encode_tuple_array():
"""Test encoding of primitive arrays."""
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says "Test encoding of primitive arrays" but this test is actually testing tuple encoding. Update the docstring to accurately describe what the test is doing, for example: "Test encoding of tuples".

Suggested change
"""Test encoding of primitive arrays."""
"""Test encoding of tuples."""

Copilot uses AI. Check for mistakes.
# Number tuple
assert encode({'numbers': (1, 2, 3)}) == 'numbers: [1,2,3]'

# String tuple
assert encode({'names': ('Alice', 'Bob')}) == 'names: [Alice,Bob]'

# Mixed primitive tuple
assert encode({'mixed': (1, 'text', True, None)}) == 'mixed: [1,text,true,null]'
Comment on lines +91 to +100
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test coverage for tuple encoding is insufficient. It only tests basic primitive tuples but doesn't cover important edge cases such as: 1) tuples with nested structures (tuples containing dicts, lists, or other tuples), 2) tuples with strings that need quoting, 3) empty tuples (which is tested in the implementation but not verified), 4) tuples used with different delimiter options (tab, pipe), and 5) tuples nested within objects or arrays. Add tests for these scenarios to ensure robust tuple encoding.

Copilot uses AI. Check for mistakes.


def test_encode_array_delimiter():
"""Test different array delimiters."""
Expand Down
16 changes: 14 additions & 2 deletions toon/encoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,11 @@ def _encode_value(value: Any, level: int, opts: EncoderOptions) -> str:
return _encode_array(value, level, opts)
elif isinstance(value, dict):
return _encode_object(value, level, opts)
elif isinstance(value, tuple):
return _encode_tuple(value)
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _encode_tuple function is called from _encode_value without passing the level and opts parameters (line 121), but _encode_tuple doesn't accept these parameters. This prevents tuple encoding from being consistent with the rest of the encoding system. The function signature should be updated to match the pattern used by _encode_array: _encode_tuple(value: tuple, level: int, opts: EncoderOptions).

Suggested change
return _encode_tuple(value)
return _encode_tuple(value, level, opts)

Copilot uses AI. Check for mistakes.
else:
# Handle other types as null
return 'null'
# Handle other types with NotImplementedError
raise NotImplementedError(f'Encoding for type {type(value)} is not implemented.')
Comment on lines 122 to +124
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR changes the behavior from encoding unknown types as 'null' to raising NotImplementedError, but there are no tests to verify this new behavior. Add a test that verifies NotImplementedError is raised for unsupported types (e.g., custom classes, sets, frozensets, etc.) to ensure the error handling works as expected.

Copilot uses AI. Check for mistakes.


def _encode_object(obj: dict, level: int, opts: EncoderOptions) -> str:
Expand Down Expand Up @@ -177,6 +179,16 @@ def _encode_array(arr: list, level: int, opts: EncoderOptions) -> str:
# Mixed array (list format)
return _encode_list_array(arr, level, opts, key=None)

def _encode_tuple(value: tuple) -> str:
"""Encode a tuple."""
if not value:
return '[]'

tuple_data = ','.join(_encode_primitive_value(v) for v in value)
Comment on lines +182 to +187
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _encode_tuple function doesn't accept or use the opts parameter, so it always uses comma as the delimiter. This creates an inconsistency with list encoding which respects the delimiter option. For example, if a user encodes data with delimiter='tab', lists will use tabs but tuples will still use commas. The function should accept level and opts parameters like _encode_array does, and either call _encode_array or reimplement the logic to respect the delimiter option.

Suggested change
def _encode_tuple(value: tuple) -> str:
"""Encode a tuple."""
if not value:
return '[]'
tuple_data = ','.join(_encode_primitive_value(v) for v in value)
def _encode_tuple(value: tuple, level: int = 0, opts: Optional[EncoderOptions] = None) -> str:
"""Encode a tuple."""
if not value:
return '[]'
# Use the configured delimiter when options are provided; fall back to comma for backward compatibility
delimiter = opts.delimiter if opts is not None else COMMA
tuple_data = delimiter.join(_encode_primitive_value(v) for v in value)

Copilot uses AI. Check for mistakes.
Comment on lines +183 to +187
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _encode_tuple function only handles primitive values using _encode_primitive_value, which falls back to 'null' for non-primitive types (line 304 in encoder.py). This means tuples containing nested structures (like dicts, lists, or other tuples) will be incorrectly encoded. For example, a tuple like ({'a': 1}, [2, 3]) would be encoded as [null,null] instead of properly encoding the nested structures. Consider converting the tuple to a list and using _encode_array instead, or implement proper handling for nested structures.

Suggested change
"""Encode a tuple."""
if not value:
return '[]'
tuple_data = ','.join(_encode_primitive_value(v) for v in value)
"""Encode a tuple.
Tuples are encoded similarly to arrays. Primitive elements are encoded
via `_encode_primitive_value`, while nested containers (lists, dicts,
and tuples) are encoded using the corresponding encoder functions.
"""
if not value:
return '[]'
# Use default encoder options for nested structures. The previous
# implementation ignored options entirely, so this is backward
# compatible while allowing proper nested encoding.
opts = EncoderOptions()
encoded_items: List[str] = []
for v in value:
if is_primitive(v):
encoded_items.append(_encode_primitive_value(v))
elif isinstance(v, list):
# Encode lists using the standard array encoder.
encoded_items.append(_encode_array(v, level=0, opts=opts))
elif isinstance(v, dict):
# Encode dictionaries as objects at the top level.
encoded_items.append(_encode_object(v, level=0, opts=opts))
elif isinstance(v, tuple):
# Recursively encode nested tuples.
encoded_items.append(_encode_tuple(v))
else:
# Fallback to primitive encoding behavior for unsupported types.
encoded_items.append(_encode_primitive_value(v))
tuple_data = ','.join(encoded_items)

Copilot uses AI. Check for mistakes.
tuple_string = f'[{tuple_data}]'

return tuple_string


def _encode_array_with_key(key: str, arr: list, level: int, opts: EncoderOptions) -> str:
"""Encode an array with its key prefix for object context."""
Expand Down
Loading