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

optional IfcCurveBoundedPlane.InnerBoundaries and cardinality 1 to many #850

Open
wants to merge 1 commit into
base: tunnel
Choose a base branch
from

Conversation

SergejMuhic
Copy link
Collaborator

closes #660

implementing proposal from @iegorychev

@SergejMuhic SergejMuhic added the EXPRESS Issues or pull requests relating to EXPRESS schema label Jun 25, 2024
@SergejMuhic SergejMuhic self-assigned this Jun 25, 2024
@SergejMuhic SergejMuhic linked an issue Jun 25, 2024 that may be closed by this pull request
Copy link
Collaborator

@TLiebich TLiebich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would disagree to the proposed change, since it creates a non-upward compatible change in regard to IFC4.3. The SPF notation would change from [] "empty set" to $ "non asserted set" and at least under previous considerations this is not upward compatible.

(the original SET[0:?] also comes from P42).

Personally I would recommend to wait until the next major IFC release to make this kind of changes.

@SergejMuhic
Copy link
Collaborator Author

SergejMuhic commented Jun 26, 2024

I would disagree to the proposed change, since it creates a non-upward compatible change in regard to IFC4.3. The SPF notation would change from [] "empty set" to $ "non asserted set" and at least under previous considerations this is not upward compatible.

(the original SET[0:?] also comes from P42).

Personally I would recommend to wait until the next major IFC release to make this kind of changes.

No worries, we will revert it for the submission. This was just for the test run. Thanks for the feedback.

I understand, the problem is that SPF serialization would suffer and old files that require an empty set would fail in deserialization since according to this spec an empty list is not allowed. A question though, what about OPTIONAL SET[0:?]. This way both $ and () are allowed.

A slight correction though, part 42 actually has SET[1:?]:
image since the definition is a bit different and there is no separate outer boundary attribute. One boundary has to be always in the set. I don't know, however, if this was changed or when it was changed but the current state in part 42 seems to be one mandatory SET[1:?].

IFC is also a bit inconsistent further, apparently two entities are referencing this one curve_bounded_surface definition from part 42:

  1. IfcCurveBoundedSurface
  2. IfcCurveBoundedPlane

EDIT: Realized this one was not even merged. I was careful. Funny, I know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EXPRESS Issues or pull requests relating to EXPRESS schema
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

Non-optional SET [0:?]
2 participants