Sps003

[aothms]

Only review 1a67dff thanks

[pjanck]

I've reviewed from 1a67dff onwards.

[pjanck]

(I've left similar comments with other PRs.)

These two sentences seem to address the same aspect of a model, with the only difference being applicability vs requirement in the rule specification. I'd expect these to have the same structure, or very similar. For example:

Suggested change

	And Decomposes = not empty
	Then The value of attribute ContainedInStructure must be empty
	And Decomposes is not empty
	Then ContainedInStructure is empty

[Ghesselink]

There is the possibility to make them more similar. For example, in the given statements

@then('The value of attribute {attribute} must be {value}')
@then('{attribute} = {value}')

This way, as a rule-creator, there's the freedom to define both the given and then statement the same way.

      And Decomposes = not empty
      Then ContainedInStructure = empty

However, your suggestion of 'is' does not work unfortunately (yet). When the gherkin implementation is decorated with

@then('{attribute} is {value}'

The rule 'ALB002'
Then Each IfcAlignmentHorizontal is nested by a list of only instance(s) of IfcAlignmentSegment
Will match that the following as the value in the given statement; 'nested by a list of only instance(s) of IfcAlignmentSegment

[aothms]

I fully agree. In the interest of time I'd like to propose to address this at a later stage, because I think it's only a symptom of a deeper problem that:

• because gherkin only matches full sentences we can't really do meaningful composition on a subsentence level. @Ghesselink did nice experiments with using proper grammars instead of just placeholder pattern matching. And ultimately I find that much more promising. Because you define once what a 'value' is (e.g do we use a programming like literal, or something else) we couple it once with a transformer to convert it to a python object and we can use it in all sentences.
• because of the way of how we log errors and transform applicability state we can't really reuse statements among applicability and requirements. Whereas I would really expect them to be a function F(instances) -> {True, False}. When used as an applicability the set of instances gets larger or smaller, when used as a requirement, True triggers a rule_passed event, False triggers an error. This is actually quite easily solvable. We just define our own decorator with the sentence statement as well as an error message around a function returning true or false. And in this decorator we actually delegate to @then and @given. This would be a great step forward, but the problem with tracking applicability remains.

I think we've learned a lot and both the features + the implementations are useful. But the matching and reporting provided by gherkin is really a limiting factor, but also not insurmountable, and maybe it also forces us to limit complexity to a level that people can still grok. We just need to have a solid discussion on what direction we want to take and then we can unify the sentences we currently have.

[pjanck]

Edge case: what is I wanted the attribute to have the value "empty"? How would this work?

[aothms]

In this case it's actually possible. Because that string value would have to be a valid python literal "empty" (unwrapped by ast.literal_eval()). See comment below, where this edge case is real, because we don't pass around python literals, so can't differentiate between "empty" and empty. To be addressed later.

[pjanck]

See also my comment above and the other PRs. The question remains: should this be solved before merging?

What is ast?

[Ghesselink]

the horror and inconsistency

I know, hehe ..

In response to Stefan's comment, it might be beneficial to consider reorganizing the structure. Broadly speaking, when parsing Gherkin statements related to values and attributes, it seems we encounter two distinct scenarios:

1. Direct value-to-attribute linkage, as observed here, such as "The type of the attribute 'Items' must be IfcCurveSegment."
2. Through relationships: mapping from one entity to another, like "An IfcAlignmentSegment nests an IfcAlignmentHorizontalSegment". This can further have a conditional statement: "value -> relationship -> entity -> condition".

Intuitively, I think this approach would simplify the process by parsing the information into more manageable chunks, leading to more consistent and replicable code. This would consolidate relationship terms (like nesting, containing, associating) into one location alongside the entities (e.g., IfcRelAssigns, IfcRelAssociates, etc.). The conditions can be categorized separately, encompassing clauses like "if X is present", "a list of only", or "num instances of".

Regarding the condition to a 'given' statement. Currently it's kind of mixed, perhaps adding to the horror and inconsistency. For instance

      Given A file with Model View Definition "CoordinationView"
        And A file with Schema Identifier "IFC2X3"

       Then There must be at least 1 instance(s) of IfcBuilding
        And The IfcBuilding must be assigned to the IfcSite if IfcSite is present
        And The IfcBuilding must be assigned to the IfcProject if IfcSite is not present

Change to

 Background: 
  Given A file with Model View Definition "CoordinationView"
    And A file with Schema Identifier "IFC2X3"

  Scenario 1 : The IfcBuilding must be assigned to the IfcSite if IfcSite is present
   Given IfcSite is present # condition 
   Then A relationship from IfcBuilding to IfcSite  # entity -> rel -> entity
   And The relationship is must be 'assigned'

   Scenario 2: # The IfcBuilding must be assigned to the IfcProject if IfcSite is not present
  Given IfcSite is not present
  Then  A relationship from IfcBuilding to IfcSite # or to/from, as @aothms implemented before
   And The relationship is must be 'assigned' # entity -> rel -> entity

  Scenario 3: 
   Then There must be a least 1 instance(s) of IfcBuilding

Or

      And An IfcAlignment
      Then Each IfcAlignment must be directly contained in IfcSite

To

Given An IfcAlignment
And A relationship from IfcAlignment to IfcSite
Then The relationship is 'directly contained'

A potential addition could involve adding an extra table to the database that stores previously identified relationships between two entities. This could optimize the search process; if we already know that the relationship between IfcEntity1 and IfcEntity2 is defined as ["X"], the system can begin its search based on this established connection.

[Ghesselink]

What is ast?

I believe it refers to a module to parse string into the correct python datatype. For instance

>>> example_tuple = "('thomas','stefan','geert')"
>>> type(example_tuple)
<class 'str'>
>>> example_tuple[1]
"'"
>>> type(ast.literal_eval(example_tuple))
<class 'tuple'>
>>> ast.literal_eval(example_tuple)[1]
'stefan'

[aothms]

@Ghesselink take note of these things, we should have a well-prepared session on moving forward. For now not a lot we can do.