Remove cardinality checks for attributes

[pasi-paasiala]

It is possible to specify cardinality for attributes as shown in this example.

The specific requirement I'm talking about is this:

                <attribute minOccurs="0" maxOccurs="1">
                    <name>
                        <simpleValue>Description</simpleValue>
                    </name>
                    <value>
                        <simpleValue>Foobar</simpleValue>
                    </value>
                </attribute>

The interpretation of this is vague. I'd remove the possibility to specify cardinality for attributes. If IDS says something about an attribute, it is a requirement, not optional.

[CBenghi]

If @pasi-paasiala 's proposal is agreeable, which I support, just merge PR #149.

[Moult]

Why is the interpretation vague? There are clear examples here https://github.com/buildingSMART/IDS/blob/master/Documentation/testcases-ids.md and the meaning seems clear to me https://github.com/buildingSMART/IDS/blob/master/Documentation/developer-guide.md#optionality

[pjanck]

Vagueness

Why is the interpretation vague?

Does an optional requirement on an optional attribute specify anything beyond what is already defined in the IFC schema itself? Can it be safely ignored without worry to invalidate the result of a check? What if the value is not Foobar as defined in the requirement, but Barfoo? Should that fail the check?

Examples

All of the referenced examples use:

which is not according to the referenced specification:

What was the originating intention? I believe these should be addressed in #148 (or other PR) as well.

[CBenghi]

I believe these should be addressed in #148 (or other PR) as well.

I try to keep the development files up to date with the schema. Once this change is accepted I will produce a PR for those files.

[Moult]

The testcases need updating, but I think the intention is spelled out already:

https://github.com/buildingSMART/IDS/blob/master/Documentation/testcases-ids.md#pass-specification-optionality-and-facet-optionality-can-be-combined

https://github.com/buildingSMART/IDS/blob/master/Documentation/testcases-ids.md#pass-a-prohibited-specification-and-a-prohibited-facet-results-in-a-double-negative

Also notably, the occurs can be used to set prohibited. So you can say "This entity shall not have this attribute filled out".

[andyward]

Presumably the integration test case data using attribute facets also need amending? i.e those IDS files at https://github.com/buildingSMART/IDS/tree/master/Documentation/testcases/attribute

[CBenghi]

Hi @andyward, they absolutely do, but right now they are generated programmatically from code in a different repository. AFAIK only @Moult can do that. Claudio

[CBenghi]

Also notably, the occurs can be used to set prohibited. So you can say "This entity shall not have this attribute filled out".

I don't think we encountered this requirement in any of the use cases.

[aothms]

Also notably, the occurs can be used to set prohibited. So you can say "This entity shall not have this attribute filled out".

I don't think we encountered this requirement in any of the use cases.

The discussion on prohibited came fairly late. I can't reconstruct it entirely either, but I think it makes sense to be consistent here. If we want to enable forbidding certain entity usage, certain property usage, we can also prohibit attribute usage.

It makes sense that the use cases we have focus on what they need and not what they do not need. But the act of forbidding a certain attribute constructs can be very informative, such as: "IfcSite.RefLatitude should not be used, rather use IfcMapConversion".

So for me, setting maxoccurs=0 (prohibited) or minoccurs=1 (required) on an optional attribute makes sense.

facet specification with minoccurs=0 (optional) is a general recurring topic, we apparently had the desire to make optional statements: "if the tool has this data please provide it here".

For me this warrants maintaining the attribute occurs constraint as we have it, but I agree there is more effort needed in spelling out all the implications, because I agree the majority of permutations of attribute types and occurrence constraints are pure noise. But that's the consequence of the choice we made to model prohibited/optional using integers.

[CBenghi]

TLDR: We need to discuss whether to bring back <xs:attributeGroup ref="xs:occurs"/> for attributes

Long:

Your argument makes sense to me. We might have jumped the gun writing the PR. In fairness, even if I don't necessarily subscribe to it, the main argument held throughout the development was the prominence of documented use cases.

But that's the consequence of the choice we made to model prohibited/optional using integers.

Indeed... the original sin! ;-)

It would be nice to resume a regular meeting to bring IDS to a robust 1.0 in the medium short time-frame. It could be the right venue to come to sound decisions on the closure of issues and merging of PRs.

[Andrej730]

Any updates on this?

[Moult]

Can we please bring back the attribute cardinality? I really don't get the logic for this. There are clear test cases and documentation describing how it is interpreted.

[CBenghi]

The preliminary agreement on the call was as follows:

Document that cardinality applies to the value, not the existence of the attribute itself in the model. Agreement is to bring back the attribute cardinality to express the possibility of prohibiting values. Also document with at least one test case. This will require a schema change. 0.9.7 in dev.

However, the conversation moved to a bigger issue of the interpretation of the cardinality on specification, that we need to close before we make any progress here. See #203

[CBenghi]

Added to the schema changes presented in the call on 2024-02-02