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

Change names of some "literal" production because they are not stricly literals. #790

Merged
merged 9 commits into from
Mar 13, 2023

Conversation

pvretano
Copy link
Contributor

Change "instantLiteral" to "instantInstance", "temporalLiteral" to "temporalInstance", "spatialLiteral" to "spatialInstance" and "arrayLiteral" to "arrayInstance".

…emporalInstance", "spatialLiteral" to "spatialInstance" and "arrayLiteral" to "arrayInstance".
Copy link
Member

@cportele cportele left a comment

Choose a reason for hiding this comment

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

I have updated a few more places. Please check and merge, if my edits make sense.

@cportele
Copy link
Member

Meeting 2023-02-27: @pvretano will review and merge today.

@jerstlouis
Copy link
Member

jerstlouis commented Mar 10, 2023

The only thing here that is not consistent with the conceptual model for Styles & Symbology is the arrayInstance.

Arrays and Instances are two fundamental types of expressions in it.
In programming languages, arrays are often implemented as some kind of container class, so it is not entirely distinct, but in JSON for example you use [ ] for arrays and { } for instances.

Could we simply call them arrays or arrayExpressions instead of arrayInstances, to avoid confusion?

From 7.1.4. Expressions:

download

download

For the spatial instances, they are essentially regular instances of a base Geometry class in that model.
See 18.2.1.2 Geometry.

Somewhat related to all this, is recent consideration in extending CQL2 for the Styles & Symbology encoding, and harmonizing WKT (which uses tuples without commas), INTERVAL (with start end separated by comma), the UML model that e.g., says that a POINT has a coordinates array, the need for explicitly setting properties and the css <property>: value syntax for instances. Also perhaps allowing { } for instances and [ ] for arrays as an alternative there, for clarity.
See 21.1.6. Expressions for a lot of details about these considerations.

@pvretano
Copy link
Contributor Author

pvretano commented Mar 10, 2023

Just so I understand ... @jerstlouis you want me to rename arrayInstance to either array or arrayExpression. Is that correct? Also you want me to allow { } for instances ... instances of what? Also, you want me to allow arrays to be delimeted by either ( ) or [ ] ... is that correct?

I reviewed 21.1.6 Expression and my initial comment is that people agreed to this? It seem like you are allowing multiple ways to encode the same thing which I think is a bad idea. Also, it seems you are trying to shoehorn CQL2 into CSS which, again, seems a little awkward.

@jerstlouis
Copy link
Member

jerstlouis commented Mar 10, 2023

@pvretano

you want me to rename arrayInstance to either array or arrayExpression. Is that correct?

That is correct.

Also you want me to allow { } for instances ... instances of what? Also, you want me to allow arrays to be delimeted by either ( ) or [ ] ... is that correct?

Not in vanilla CQL2, at least not 1.0 (unless everyone think it's a good idea to make this change already?).
That is an open question for consideration right now in Styles & Symbology.

instances of what?

With Styles and Symbology, as in #705, the idea is that from the grammar perspective, it doesn't matter instance of what.
Could be a Point, a Polygon, a Color, an Interval, an Instant, a user-defined class from a complex schema. The syntax for an instantiation is consistent (and the same grammar production rule): <class name> ( <member initializers> ) or <class name> { <member initializers> }.

I reviewed 21.1.6 Expression and my initial comment is that people agreed to this? It seem like you are allowing multiple ways to encode the same thing which I think is a bad idea.

That is a current topic of discussion in Styles & Symbology. We want to use and extend CQL2, but this is the logical conclusion of maintaining consistency with WKT, INTERVAL, and arbitrary instance expressions. Note that this is somewhat related to the complex object support e.g., for Search/Queries.

Also, it seems you are trying to shoehorn CQL2 into CSS which, again, seems a little awkward.

Note that this is Cascading Cartographic Symbology Style Sheets and not Web-CSS.
We want to leverage CQL2, and we want the cascading aspect and general convenience of Web-CSS, and trying to figure out the best approach to extend CQL2 for that purpose in a way that feels natural.

I am accutely aware of the multiple ways to encode the same thing which may seem like a bad idea, but so far this is the best we could come up with :) Ideas and suggestions welcome.

P.S. another place where the CSS-like syntax <member>: <value> appears useful is to define/name derived fields with properties=:

properties=ndvi:(B08-B04)/(B08+B04),r:B04,g:B03,b:B02

Change `arrayInstance` to `array`.
Change `arrayInstance` to `array`.
Change `arrayInstance` to `array`.
Change `arrayInstance` to `array`.
Change `arrayInstance` to `array`.
Change `arrayInstance` to `array`.
@pvretano
Copy link
Contributor Author

@cportele I changed arrayInstance to array as per the discusion above with @jerstlouis. If no one objects at the 13-MAR-2023 SWG meeting then please go ahead and merge the PR. Didn't want to unilaterally merge without people being aware of this -- albeit small -- change.

@cportele
Copy link
Member

@pvretano - I reviewed the changes, looks good to me.

@cportele cportele merged commit fba9810 into opengeospatial:master Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants