Skip to content
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.

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

Compound Fields only for Iterables #1070

Open
skinkie opened this issue Aug 25, 2024 · 4 comments · May be fixed by #1073
Open

Compound Fields only for Iterables #1070

skinkie opened this issue Aug 25, 2024 · 4 comments · May be fixed by #1073

Comments

@skinkie
Copy link
Contributor

skinkie commented Aug 25, 2024

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
Copy link
Owner

tefra commented Aug 25, 2024

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 #737

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

@skinkie
Copy link
Contributor Author

skinkie commented Aug 25, 2024

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. (XsData ignores some choice elements #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
Copy link
Owner

tefra commented Aug 25, 2024

Thanks for giving a simple example not from netex

@skinkie
Copy link
Contributor Author

skinkie commented Aug 25, 2024

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?

skinkie added a commit to skinkie/xsdata that referenced this issue Sep 4, 2024
@skinkie skinkie linked a pull request Sep 4, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants