tefra / xsdata

Naive XML & JSON Bindings for python
https://xsdata.readthedocs.io
MIT License
318 stars 57 forks source link

Compound Fields only for Iterables #1070

Open skinkie opened 3 weeks ago

skinkie commented 3 weeks ago

A follow up on #1065.

CompoundFields currently aggregate choices, which allows it to render a choice item in the found order. In the regular case where an object has two fields, and either one of those fields must be set (or read) I would prefer to access that field by its field name, not field1_field2 or choice. There is only one situation - I am aware of - where I want to maintain order: this is when the choice field is part of a sequence with maxOccurs > 1, hence it becomes a List.

To make it very concrete, first is with compound fields, the second is not. I wish that if and only if default_factory is a list, the compound code would be applied. I'll obviously give it a shot.

@dataclass(kw_only=True)
class PointsInJourneyPatternRelStructure(StrictContainmentAggregationStructure):
    class Meta:
        name = "pointsInJourneyPattern_RelStructure"

    point_in_journey_pattern_or_stop_point_in_journey_pattern_or_timing_point_in_journey_pattern: List[Union[PointInJourneyPattern, StopPointInJourneyPattern, TimingPointInJourneyPattern]] = field(
        default_factory=list,
        metadata={
            "type": "Elements",
            "choices": (
                {
                    "name": "PointInJourneyPattern",
                    "type": PointInJourneyPattern,
                    "namespace": "http://www.netex.org.uk/netex",
                },
                {
                    "name": "StopPointInJourneyPattern",
                    "type": StopPointInJourneyPattern,
                    "namespace": "http://www.netex.org.uk/netex",
                },
                {
                    "name": "TimingPointInJourneyPattern",
                    "type": TimingPointInJourneyPattern,
                    "namespace": "http://www.netex.org.uk/netex",
                },
            ),
            "min_occurs": 2,
        },
    )
@dataclass(kw_only=True)
class PointsInJourneyPatternRelStructure(StrictContainmentAggregationStructure):
    class Meta:
        name = "pointsInJourneyPattern_RelStructure"

    point_in_journey_pattern: List[PointInJourneyPattern] = field(
        default_factory=list,
        metadata={
            "name": "PointInJourneyPattern",
            "type": "Element",
            "namespace": "http://www.netex.org.uk/netex",
            "min_occurs": 2,
        },
    )
    stop_point_in_journey_pattern: List[StopPointInJourneyPattern] = field(
        default_factory=list,
        metadata={
            "name": "StopPointInJourneyPattern",
            "type": "Element",
            "namespace": "http://www.netex.org.uk/netex",
            "min_occurs": 2,
        },
    )
    timing_point_in_journey_pattern: List[TimingPointInJourneyPattern] = field(
        default_factory=list,
        metadata={
            "name": "TimingPointInJourneyPattern",
            "type": "Element",
            "namespace": "http://www.netex.org.uk/netex",
            "min_occurs": 2,
        },
    )
tefra commented 3 weeks ago

I am not sure I follow @skinkie, you are suggesting to skip the compound field if there is no array element in the choices right? The example you give is not very accurate.

This was the default behavior, but the counter-argument is that but having flat fields there is no hint that one of the fields must be present https://github.com/tefra/xsdata/issues/737

If I am way off please provide a simple xsd example, not netex 😄

skinkie commented 2 weeks ago

What I want to achieve is that unless it is a list of choices, opposed to a choice of lists, individual field access is possible. I even agree that the bug/behavior reported by #737 is what I intend for everything that is not a list. None of the examples in #737 are actually an ordered list of elements where the elements in the list may be a choice.

Going to give an NeTEx example anyway ;-) I can simplyfy this, if we need a test. So the choice itself here is the element that is maxOccurs > 1. From my perspective that is the only time I care about order.

<xsd:complexType name="pointsInJourneyPattern_RelStructure" abstract="false">
        <xsd:annotation>
                <xsd:documentation>Type for POINT IN JOURNEY PATTERN.</xsd:documentation>
        </xsd:annotation>
        <xsd:complexContent>
                <xsd:extension base="strictContainmentAggregationStructure">
                        <xsd:choice minOccurs="2" maxOccurs="unbounded">
                                <xsd:element ref="PointInJourneyPattern"/>
                                <xsd:element ref="StopPointInJourneyPattern"/>
                                <xsd:element ref="TimingPointInJourneyPattern"/>
                        </xsd:choice>
                </xsd:extension>
        </xsd:complexContent>
</xsd:complexType>

@martinwiesmann is not wrong with the statement "This class definition does not include any hint that only one of these elements are allowed." I think fundamentally that was what #1065 about, the choice is there, there can be only one value be present, but I would like to see that individual field names remain available.

From my perspective the compound fields implementation is mixing two behaviors.

  1. It joins choice fields into a single attribute. (#737) (attribute1_or_attribute2_or_attribute3)
  2. It joins repeated choice fields into single attributes, to facilitate retaining order. (ordered_list_consisting_of_attribute1_or_attribute2_or_attribute3)

I am not disputing 1 and 2 should be done, 1 gives me something in practice is very difficult to write code for. The generation results in always changing code if anything ot the choice changes and replacing the choice attribute name with choice is even worse for readibility.

Your suggestion was to disable compound fields. An other approach where choices in category 2 stay as-is, but for choices in category 1 something like below is made available.

block_ref exist because it originates from a substitution group, this will obviously cause issues so maybe we need to revert that back to block_ref_or_train_block_ref. We offer access to two fields for operator_ref_or_operator_view.

@dataclass(kw_only=True)
class ServiceJourneyVersionStructure(JourneyVersionStructure):
    class Meta:
        name = "ServiceJourney_VersionStructure"

    block_ref: Optional[Union[TrainBlockRef, BlockRef]] = field(
        default=None,
        metadata={
            "type": "Elements",
            "choices": (
                {
                    "name": "TrainBlockRef",
                    "type": TrainBlockRef,
                    "namespace": "http://www.netex.org.uk/netex",
                },
                {
                    "name": "BlockRef",
                    "type": BlockRef,
                    "namespace": "http://www.netex.org.uk/netex",
                },
            ),
        },
    )

    operator_ref_or_operator_view: Optional[Union[OperatorRef, OperatorView]] = field(
        default=None,
        metadata={
            "type": "Elements",
            "choices": (
                {   
                    "name": "OperatorRef",
                    "type": OperatorRef,
                    "namespace": "http://www.netex.org.uk/netex",
                },  
                {   
                    "name": "OperatorView",
                    "type": OperatorView,
                    "namespace": "http://www.netex.org.uk/netex",
                },  
            ),      
        },          
    )

    @property
    def operator_ref(self) -> OperatorRef:
        if isinstance(self.operator_ref_or_operator_view, OperatorRef):
            return self.operator_ref_or_operator_view

    @operator_ref.setter
    def operator_ref(self, v: OperatorRef) -> None:
        self.operator_ref_or_operator_view = v

    @property
    def operator_view(self) -> OperatorView:
        if isinstance(self.operator_ref_or_operator_view, OperatorView):
            return self.operator_ref_or_operator_view

    @operator_view.setter
    def operator_view(self, v: OperatorView) -> None:
        self.operator_ref_or_operator_view = v

The introduced properties do not seem to interfere with the actual XML generation.

from xsdata.formats.dataclass.serializers import XmlSerializer
from xsdata.formats.dataclass.serializers.config import SerializerConfig

from netex import ServiceJourneyVersionStructure, OperatorRef, OperatorView

x = ServiceJourneyVersionStructure(id="test", version="1")
x.operator_ref = OperatorRef(ref="x", version="1")
print(f"x.operator_ref {x.operator_ref}")
print(f"x.operator_view {x.operator_view}")
print(f"x.operator_ref_or_operator_view {x.operator_ref_or_operator_view}")

x.operator_view = OperatorView(id="view")
print(f"x.operator_ref {x.operator_ref}")
print(f"x.operator_view {x.operator_view}")
print(f"x.operator_ref_or_operator_view {x.operator_ref_or_operator_view}")

serializer_config = SerializerConfig(ignore_default_attributes=True, pretty_print=True)
serializer_config.ignore_default_attributes = True
serializer = XmlSerializer(config=serializer_config)

print(serializer.render(x))
x.operator_ref OperatorRef(value='', name_of_ref_class=None, created=None, changed=None, version='1', modification=None, ref='x', version_ref=None, uri=None)
x.operator_view None
x.operator_ref_or_operator_view OperatorRef(value='', name_of_ref_class=None, created=None, changed=None, version='1', modification=None, ref='x', version_ref=None, uri=None)
x.operator_ref None
x.operator_view OperatorView(branding_ref=None, id='view', operator_ref=None, name=None, short_name=None, legal_name=None, trading_name=None, alternative_names=None)
x.operator_ref_or_operator_view OperatorView(branding_ref=None, id='view', operator_ref=None, name=None, short_name=None, legal_name=None, trading_name=None, alternative_names=None)
<?xml version="1.0" encoding="UTF-8"?>
<ServiceJourney_VersionStructure id="test" version="1">
  <ns0:OperatorView xmlns:ns0="http://www.netex.org.uk/netex" id="view"/>
</ServiceJourney_VersionStructure>

I would call this stable compound field access. I hope that my aim is clear.

tefra commented 2 weeks ago

Thanks for giving a simple example not from netex

skinkie commented 2 weeks ago

Thanks for giving a simple example not from netex

This was from NeTEx, do you want me to copy paste an XML schema that is at most 40 lines?