opengeospatial / cartographic-symbology

OGC Cartographic Symbology
Other
11 stars 6 forks source link

Clarify inconsistencies between spec and grammar #77

Open maxcollombin opened 3 months ago

maxcollombin commented 3 months ago
jerstlouis commented 3 months ago

@maxcollombin the stylingRuleList? is the nested rules and the (propertyAssignmentList SEMI)? is the symbolizer.

Like for the CartoSym-JSON, how the grammar / schema is defined does not necessarily need to use the exact same terms as the UML, since it expresses something different.

But we could try to make this a bit more consistent/obvious, by renaming to these to e.g., nestedStylingRulesList and symbolizerPropertyAssignments ?

But the same stylingRuleList also applies the top-level styling rules of the style, that's why it's not called nestedStylingRulesList.

By name, I imagine you mean the name of the stylingRule for the legend labeling purpose? That one is missing right now.

maxcollombin commented 3 months ago

Same remark/question for the name property of the IdentifierExpression class relative to the Expression class. Does the name property correspond to a grammar element? Sorry for the obvious questions (still learningđŸ˜†)

jerstlouis commented 3 months ago

@maxcollombin

Does the name property correspond to a grammar element? Sorry for the obvious questions (still learningđŸ˜†)

There is not currently a single element corresponding to that in the CartoSym-CSS grammar.

It corresponds primarily to the IDENTIFIER when used specifically in this rule:

idOrConstant: IDENTIFIER | expConstant;

We probably need to clarify whether the . object member operator and the [ ] array indexing operators are folded in and considered part of an IdentifierExpression, or their own expression types. In our CMSS implementation, these are separate classes of expressions, but these concepts do not exist yet in CQL2.

So whether you consider expression DOT IDENTIFIER in this rule a single IdentifierExpression and that whole object.member is its name is not yet well defined (same for array[10] in this rule).

maxcollombin commented 3 months ago

It corresponds primarily to the IDENTIFIER when used specifically in this rule: idOrConstant: IDENTIFIER | expConstant;

Thanks for your feedback @jerstlouis . This is the result of a problem with my implementation that I have just reported here.

maxcollombin commented 3 months ago

I'm also having some difficulty relating this class diagram:

Figure 1 — Symbology Core Classes UML Diagram

to this part of the grammar:

///////////////////////////////
// High level style sheet rules
///////////////////////////////

styleSheet: metadata* stylingRuleList;

metadata:
    '.' IDENTIFIER CHARACTER_LITERAL;

stylingRuleList:
     stylingRule
   | stylingRuleList stylingRule;

stylingRule:
   ( selector )*
   LCBR
      (propertyAssignmentList SEMI)?
      stylingRuleList?
   RCBR;

selector:
     IDENTIFIER
   | LSBR expression RSBR ;

///////////////////////////////
maxcollombin commented 3 months ago

Regardin the Symbology Core Classes UML Diagram, I'd rather see something like this:

TL4zJyCm4DtzAwoCaLerQwPAm0P2ecF5uCP7Q-7ObUy2f1N_dNCKsQc1ASdxU7dFtYAfG4DlHMSLejpHQ65t0Y1v5LBsG4ehKdApNjUI0MJUkEFBOE7H8Cb6V8lP-ZHh9oCPij2oFB1wp-xcDTerO1VCcOntjoHpQg2J3xZ4wuY_m_ZfW_vD7C7VAr9tyUzGqgdMhcXylLtHB

maxcollombin commented 3 months ago

Here is the associated PlantUML code syntax:

StyleSheet : metadata 0..* 
StyleSheet : stylingRuleList 1

Metadata : title: string[0..1] 
Metadata : description: string[0..1]
Metadata : authors: string[0..*]
Metadata : keywords: string[0..*]
Metadata : geoDataClasses: string[0..*]

StylingRuleList : stylingRule: StylingRule[1..*]

StylingRule : name: string
StylingRule : symbolizer: Symbolizer[0..1]
StylingRule : selector: Expression[0..1]
StylingRule : nestedRules: StylingRuleList[0..1]

Symbolizer : visibility: bool
Symbolizer : opacity: float
Symbolizer : zOrder: integer

StyleSheet "1" -- "*" Metadata
StyleSheet "1" -- "1" StylingRuleList
StylingRuleList "0" *-- "*" StylingRule
StylingRule "0" *-- "1" Expression
StylingRule "0" *-- "*" Symbolizer
jerstlouis commented 3 months ago

@maxcollombin

I'd rather see something like this:

In my view, conceptually a class with only one element is unnecessary. It's cleaner to see it as a StylingRule with a 0..* multiplicity.

The stylingRuleList grammar rule is just a convenience for describing the grammar.

In my view, the grammar for the encoding does not directly relate to the conceptual / logical model at all (e.g., none of symbolizer properties / system identifier etc. appear at all in the grammar).