dapper91 / pydantic-xml

python xml for humans
https://pydantic-xml.readthedocs.io
The Unlicense
141 stars 14 forks source link

Cannot serialize optional raw fields. #158

Closed ajoino closed 5 months ago

ajoino commented 5 months ago

This class

from pydantic_xml import BaseXmlModel, element
from lxml import etree

class OptionalRaw(BaseXmlModel, arbitrary_types_allowed=True):
    optional_raw_element: etree._Element | None = element("OptionalRawElement", default=None)

can be constructed with zero arguments but fails to serialize

>>> class OptionalRaw(BaseXmlModel, arbitrary_types_allowed=True):
...     optional_raw_element: etree._Element | None = element("OptionalRawElement", default=None)
... 
>>> OptionalRaw().to_xml()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jacob/.local/lib/python3.10/site-packages/pydantic_xml/model.py", line 399, in to_xml
    return etree.tostring(self.to_xml_tree(skip_empty=skip_empty), **kwargs)
  File "/home/jacob/.local/lib/python3.10/site-packages/pydantic_xml/model.py", line 380, in to_xml_tree
    self.__xml_serializer__.serialize(
  File "/home/jacob/.local/lib/python3.10/site-packages/pydantic_xml/serializers/factories/model.py", line 170, in serialize
    field_serializer.serialize(
  File "/home/jacob/.local/lib/python3.10/site-packages/pydantic_xml/serializers/factories/raw.py", line 38, in serialize
    sub_element = element.from_native(value)
  File "/home/jacob/.local/lib/python3.10/site-packages/pydantic_xml/element/native/lxml.py", line 22, in from_native
    tag=element.tag,
AttributeError: 'NoneType' object has no attribute 'tag'

Maybe this is intended behavior, but it feels like a bug to me since every other field can be optional, so raw elements should probably be too.

EDIT:

I also tried adding nillable=True to the element but it still doesn't let me construct an instance directly:

from pydantic_xml import BaseXmlModel, element
from lxml import etree

class OptionalRaw(BaseXmlModel, arbitrary_types_allowed=True):
    optional_raw_element: etree._Element | None = element("OptionalRawElement", nillable=True)

If I explicitly set OptionalRaw(optional_raw_element=None).to_xml() I get the same message as before and if I leave it empty I get

>>> OptionalRaw().to_xml()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jacob/.local/lib/python3.10/site-packages/pydantic/main.py", line 164, in __init__
    __pydantic_self__.__pydantic_validator__.validate_python(data, self_instance=__pydantic_self__)
pydantic_core._pydantic_core.ValidationError: 1 validation error for OptionalRaw
optional_raw_element
  Field required [type=missing, input_value={}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.5/v/missing

One of the two should work here IMO.

ajoino commented 5 months ago

As is often the case, this is probably PEBCAK: I think this happens because I forgot to set to_xml(skip_empty=True). So while this isn't a bug, the library could perhaps be improved by automatically skipping Optional[etree._Element] fields whose value is None. That behaviour makes a bit more sense to me than the current one, but I'll leave @dapper91 to decide on that. I might try to implement this though but I'll hold the PR.

ajoino commented 5 months ago

A quick test suggests this patch works

--- a/pydantic_xml/serializers/factories/raw.py
+++ b/pydantic_xml/serializers/factories/raw.py
@@ -32,7 +32,7 @@ class ElementSerializer(Serializer):
     def serialize(
             self, element: XmlElementWriter, value: Any, encoded: Any, *, skip_empty: bool = False,
     ) -> Optional[XmlElementWriter]:
-        if value is None and skip_empty:
+        if value is None:
             return element

         sub_element = element.from_native(value)

No tests break, but I'm not sure if removing that check has unintended consequences. I don't think it does, but I'm not sure.

dapper91 commented 5 months ago

@ajoino Hi,

Yes, that's a bug.