chore(jambo): Remove redundant comments and docstrings (#8)
* Remove redundant comments in type parsers and tests * Clean up redundant comments in `test_oneof_type_parser.py` * Remove redundant comments across parsers and tests
This commit is contained in:
@@ -127,23 +127,9 @@ class GenericTypeParser(ABC, Generic[T]):
|
||||
|
||||
@staticmethod
|
||||
def _has_meaningful_constraints(field_props):
|
||||
"""
|
||||
Check if field properties contain meaningful constraints that require Field wrapping.
|
||||
|
||||
Returns False if:
|
||||
- field_props is None or empty
|
||||
- field_props only contains {'default': None}
|
||||
|
||||
Returns True if:
|
||||
- field_props contains a non-None default value
|
||||
- field_props contains other constraint properties (min_length, max_length, pattern, etc.)
|
||||
"""
|
||||
if not field_props:
|
||||
return False
|
||||
|
||||
# If only default is set and it's None, no meaningful constraints
|
||||
if field_props == {"default": None}:
|
||||
return False
|
||||
|
||||
# If there are multiple properties or non-None default, that's meaningful
|
||||
return True
|
||||
|
||||
@@ -35,17 +35,14 @@ class ArrayTypeParser(GenericTypeParser):
|
||||
|
||||
mapped_properties = self.mappings_properties_builder(properties, **kwargs)
|
||||
|
||||
# Only set default_factory if the field is not required OR if there's an actual default
|
||||
if not kwargs.get("required", False) and "default" not in mapped_properties:
|
||||
mapped_properties["default_factory"] = self._build_default_factory(
|
||||
properties.get("default"), wrapper_type
|
||||
)
|
||||
elif "default" in properties:
|
||||
# If there's a default value specified, set the default_factory
|
||||
mapped_properties["default_factory"] = self._build_default_factory(
|
||||
properties["default"], wrapper_type
|
||||
)
|
||||
# Remove the regular default since we're using default_factory
|
||||
mapped_properties.pop("default", None)
|
||||
|
||||
return field_type, mapped_properties
|
||||
|
||||
@@ -33,14 +33,10 @@ class ConstTypeParser(GenericTypeParser):
|
||||
return const_type, parsed_properties
|
||||
|
||||
def _build_const_type(self, const_value):
|
||||
# Try to use Literal for hashable types (required for discriminated unions)
|
||||
# Fall back to validator approach for non-hashable types
|
||||
try:
|
||||
# Test if the value is hashable (can be used in Literal)
|
||||
hash(const_value)
|
||||
return Literal[const_value]
|
||||
except TypeError:
|
||||
# Non-hashable type (like list, dict), use validator approach
|
||||
def _validate_const_value(value: Any) -> Any:
|
||||
if value != const_value:
|
||||
raise ValueError(
|
||||
|
||||
@@ -28,7 +28,7 @@ class StringTypeParser(GenericTypeParser):
|
||||
"time": time,
|
||||
"date-time": datetime,
|
||||
"binary": bytes,
|
||||
"file-path": FilePath, # Added file-path format
|
||||
"file-path": FilePath,
|
||||
}
|
||||
|
||||
format_pattern_mapping = {
|
||||
|
||||
@@ -99,16 +99,13 @@ class TestArrayTypeParser(TestCase):
|
||||
parser.from_properties("placeholder", properties)
|
||||
|
||||
def test_array_parser_required_without_default(self):
|
||||
"""Regression test: Required array fields without defaults should be required"""
|
||||
parser = ArrayTypeParser()
|
||||
|
||||
properties = {"items": {"type": "string"}}
|
||||
|
||||
# Test with required=True (should be required)
|
||||
type_parsing, type_validator = parser.from_properties(
|
||||
"test_array", properties, required=True
|
||||
)
|
||||
|
||||
# Should NOT have default_factory when required and no default specified
|
||||
self.assertNotIn("default_factory", type_validator)
|
||||
self.assertNotIn("default", type_validator)
|
||||
|
||||
@@ -7,7 +7,6 @@ from unittest import TestCase
|
||||
|
||||
class TestConstTypeParser(TestCase):
|
||||
def test_const_type_parser_hashable_value(self):
|
||||
"""Test const parser with hashable values (uses Literal)"""
|
||||
parser = ConstTypeParser()
|
||||
|
||||
expected_const_value = "United States of America"
|
||||
@@ -17,31 +16,27 @@ class TestConstTypeParser(TestCase):
|
||||
"country", properties
|
||||
)
|
||||
|
||||
# Check that we get a Literal type for hashable values
|
||||
self.assertEqual(get_origin(parsed_type), Literal)
|
||||
self.assertEqual(get_args(parsed_type), (expected_const_value,))
|
||||
|
||||
self.assertEqual(parsed_properties["default"], expected_const_value)
|
||||
|
||||
def test_const_type_parser_non_hashable_value(self):
|
||||
"""Test const parser with non-hashable values (uses Annotated with validator)"""
|
||||
parser = ConstTypeParser()
|
||||
|
||||
expected_const_value = [1, 2, 3] # Lists are not hashable
|
||||
expected_const_value = [1, 2, 3]
|
||||
properties = {"const": expected_const_value}
|
||||
|
||||
parsed_type, parsed_properties = parser.from_properties_impl(
|
||||
"list_const", properties
|
||||
)
|
||||
|
||||
# Check that we get an Annotated type for non-hashable values
|
||||
self.assertEqual(get_origin(parsed_type), Annotated)
|
||||
self.assertIn(list, get_args(parsed_type))
|
||||
|
||||
self.assertEqual(parsed_properties["default"], expected_const_value)
|
||||
|
||||
def test_const_type_parser_integer_value(self):
|
||||
"""Test const parser with integer values (uses Literal)"""
|
||||
parser = ConstTypeParser()
|
||||
|
||||
expected_const_value = 42
|
||||
@@ -51,14 +46,12 @@ class TestConstTypeParser(TestCase):
|
||||
"int_const", properties
|
||||
)
|
||||
|
||||
# Check that we get a Literal type for hashable values
|
||||
self.assertEqual(get_origin(parsed_type), Literal)
|
||||
self.assertEqual(get_args(parsed_type), (expected_const_value,))
|
||||
|
||||
self.assertEqual(parsed_properties["default"], expected_const_value)
|
||||
|
||||
def test_const_type_parser_boolean_value(self):
|
||||
"""Test const parser with boolean values (uses Literal)"""
|
||||
parser = ConstTypeParser()
|
||||
|
||||
expected_const_value = True
|
||||
@@ -68,7 +61,6 @@ class TestConstTypeParser(TestCase):
|
||||
"bool_const", properties
|
||||
)
|
||||
|
||||
# Check that we get a Literal type for hashable values
|
||||
self.assertEqual(get_origin(parsed_type), Literal)
|
||||
self.assertEqual(get_args(parsed_type), (expected_const_value,))
|
||||
|
||||
|
||||
@@ -351,32 +351,26 @@ class TestOneOfTypeParser(TestCase):
|
||||
}
|
||||
}
|
||||
],
|
||||
"discriminator": {} # discriminator without propertyName
|
||||
"discriminator": {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Model = SchemaConverter.build(schema)
|
||||
|
||||
# Should succeed because input matches exactly one schema (the first one)
|
||||
# The first schema matches: type="a" matches const("a"), value="test" is a string
|
||||
# The second schema doesn't match: type="a" does not match const("b")
|
||||
obj = Model(value={"type": "a", "value": "test", "extra": "invalid"})
|
||||
self.assertEqual(obj.value.type, "a")
|
||||
self.assertEqual(obj.value.value, "test")
|
||||
|
||||
# Test with input that matches the second schema
|
||||
obj2 = Model(value={"type": "b", "value": 42})
|
||||
self.assertEqual(obj2.value.type, "b")
|
||||
self.assertEqual(obj2.value.value, 42)
|
||||
|
||||
# Test with input that matches neither schema (should fail)
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
Model(value={"type": "c", "value": "test"})
|
||||
self.assertIn("does not match any of the oneOf schemas", str(cm.exception))
|
||||
|
||||
def test_oneof_multiple_matches_without_discriminator(self):
|
||||
"""Test case where input genuinely matches multiple oneOf schemas"""
|
||||
schema = {
|
||||
"title": "Test",
|
||||
"type": "object",
|
||||
@@ -397,21 +391,18 @@ class TestOneOfTypeParser(TestCase):
|
||||
}
|
||||
}
|
||||
],
|
||||
"discriminator": {} # discriminator without propertyName
|
||||
"discriminator": {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Model = SchemaConverter.build(schema)
|
||||
|
||||
# This input matches both schemas since both accept data as string
|
||||
# and neither requires specific additional properties
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
Model(value={"data": "test"})
|
||||
self.assertIn("matches multiple oneOf schemas", str(cm.exception))
|
||||
|
||||
def test_oneof_overlapping_strings_from_docs(self):
|
||||
"""Test the overlapping strings example from documentation"""
|
||||
schema = {
|
||||
"title": "SimpleExample",
|
||||
"type": "object",
|
||||
@@ -428,21 +419,17 @@ class TestOneOfTypeParser(TestCase):
|
||||
|
||||
Model = SchemaConverter.build(schema)
|
||||
|
||||
# Valid: Short string (matches first schema only)
|
||||
obj1 = Model(value="hi")
|
||||
self.assertEqual(obj1.value, "hi")
|
||||
|
||||
# Valid: Long string (matches second schema only)
|
||||
obj2 = Model(value="very long string")
|
||||
self.assertEqual(obj2.value, "very long string")
|
||||
|
||||
# Invalid: Medium string (matches BOTH schemas - violates oneOf)
|
||||
with self.assertRaises(ValueError) as cm:
|
||||
Model(value="hello") # 5 chars: matches maxLength=6 AND minLength=4
|
||||
self.assertIn("matches multiple oneOf schemas", str(cm.exception))
|
||||
|
||||
def test_oneof_shapes_discriminator_from_docs(self):
|
||||
"""Test the shapes discriminator example from documentation"""
|
||||
schema = {
|
||||
"title": "Shape",
|
||||
"type": "object",
|
||||
@@ -477,17 +464,14 @@ class TestOneOfTypeParser(TestCase):
|
||||
|
||||
Model = SchemaConverter.build(schema)
|
||||
|
||||
# Valid: Circle
|
||||
circle = Model(shape={"type": "circle", "radius": 5.0})
|
||||
self.assertEqual(circle.shape.type, "circle")
|
||||
self.assertEqual(circle.shape.radius, 5.0)
|
||||
|
||||
# Valid: Rectangle
|
||||
rectangle = Model(shape={"type": "rectangle", "width": 10, "height": 20})
|
||||
self.assertEqual(rectangle.shape.type, "rectangle")
|
||||
self.assertEqual(rectangle.shape.width, 10)
|
||||
self.assertEqual(rectangle.shape.height, 20)
|
||||
|
||||
# Invalid: Wrong properties for the type
|
||||
with self.assertRaises(ValueError):
|
||||
Model(shape={"type": "circle", "width": 10})
|
||||
|
||||
Reference in New Issue
Block a user