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

Nested lists in 0009-append-flatten #7

Closed
opatrascoiu opened this issue Jun 30, 2017 · 8 comments
Closed

Nested lists in 0009-append-flatten #7

opatrascoiu opened this issue Jun 30, 2017 · 8 comments

Comments

@opatrascoiu
Copy link
Contributor

opatrascoiu commented Jun 30, 2017

  1. Decision literalNestedList has type List<List> but returns a literal that is not compatible with its type. See below.
              <literalExpression>
			<text>[["w","x"],"y","z"]</text>
              </literalExpression>

We should change the decision to return

              <literalExpression>
			<text>[["w","x"],["y","z"]]</text>
	      </literalExpression>

and the tests accordingly.

  1. Decision append1 has type List<List>. The returned expression
		<literalExpression>
			<text>append(simpleList,literalSimpleList)</text>
		</literalExpression>

produces and output incompatible with the decision type.

We should replace the literal expression with

		<literalExpression>
			<text>append(nestedList,literalSimpleList)</text>
		</literalExpression>

and change tests accordingly.

@etirelli
Copy link
Contributor

This is actually an ambiguity in the spec, as it says that any single element is equal to a list with that single element and vice-versa. So:

a = [a]
[a] = a

That implies that:

[["w","x"],"y","z"] = [["w","x"],["y"],["z"]]

Having said that, I think it is a bad decision for the spec to allow it, and I agree we should simplify the tests here.

etirelli pushed a commit to etirelli/tck that referenced this issue Jul 29, 2017
@etirelli
Copy link
Contributor

PR #21 submitted

@agilepro
Copy link
Contributor

agilepro commented Aug 1, 2017

I am actually concerned about this.

I agree that the spec describe something I consider to be a bad idea. But the spec is the spec, and should be unambiguous. In this area it is not ambiguous ... it clearly says that a list of one element is equal to that element, then we need a test of exactly that. If the spec says one thing, but all the implementations consistently do something else, we run the risk that someone who implements correctly is incompatible with the rest of the field. The problem with this should be obvious.

While is it OK to change this existing test to "properly nest the lists" (because that is still a valid test) we should not fail to add a test that tests "improper" nesting of lists. Our goal is to show conformance with the spec, and without such a test we fail to assure conformance.

An alternate approach is to document "deviances" from the spec -- the "vendors" correction to the spec that reflects how real implementations are done. That is a dangerous path because we have to promote and maintain this unofficial modification as an adjunct to the spec. It also has the effect of sweeping the problem under the rug -- avoiding the problem instead of addressing the problem. I think instead we should simply have tests that show up as failures until the RTF changes the official spec.

@arthware
Copy link
Contributor

arthware commented Aug 1, 2017

Daniel Thanner and me discussed that in the ACTICO office the other day too. We basically agree with @agilepro s opinion. The spec is clear on that subject and we think, that the tests are valid, because they exactly reflect what the spec specifies and therefore should stay in the TCK.

I think it is very natural for business users to have that behavior: An element is equal to a list of size 1 containing that element. In reality we don't differentiate on that either: I don't care if my peach is a peach or a 'list' of peaches with 1 element containing that peach (whatever that would be) ;-)

IMHO the TCK should test exactly such cases: features and behavior the spec specifies clearly, without much room for vendor-specific interpretation.

So, if that is getting 'fixed' in the mentioned test-case, we would vote for adding another testcase, which tests the beforementioned 'list-1-element-equality' behavior, because it is part of the spec (and we even think a nice aspect of the spec for business users).

@opatrascoiu
Copy link
Contributor Author

I think some list literals have ambiguous semantics. According to the DMN standard

A list is written using square brackets to delimit the list, and commas to separate the list items (grammar rule 56). A singleton list is equal to its single item, i.e., [e]=e for all FEEL expressions e.

Let's consider the following literal

[["a"], "b"]

What is the type of the literal? If we consider ["a"] = "a" then the type is List<String>. If we consider "b"= ["b"] then the type is List<List<String>>

AFAIK the only language with a similar feature is OCL but the implicit conversion does not apply to literals, only to navigation expression.

I think we should follow the same approach and restrict e =[e] to meaningful use cases (e.g. filters).

agilepro pushed a commit that referenced this issue Aug 7, 2017
we agreed friday to merge this change, and then to add another test for irregular list nesting
@arthware
Copy link
Contributor

As the related PR has been merged, I guess this one can be closed?

@etirelli
Copy link
Contributor

@arthware the agreement from the call is that we will create a new set of tests to test specifically what was discussed here, i.e., a=[a] cases. Lets keep this ticket open for a few more days until it is done.

@etirelli
Copy link
Contributor

Closing this ticket as the original tests were simplified and a new test was added to check for [a]=a.

tarilabs added a commit that referenced this issue Oct 14, 2021
* DMN 1.3 equality

* function equality commented out

* P0D == P0Y test commented out as per TCK discussion

* DMN1-3 equality.  Uncommented time / date / datetime tests and added some more.

* DMN1-3 equality.  Uncommented time / date / datetime tests and added some more.

* DMN1-3 equality.  Provided null as info req to some range tests. Resolved duplicate test name.

* DMN1-3 equality.  Resolved (another) duplicate test name.

* Removed trailing space in element name.

* Fix DMN and Test case xml compliance /pull/389 (#7)

* fix InputData var typeRef

ref #389 (comment)

* test case Decisions w/ informationReq on InputData

fix in supplying the necessary informationRequirement

ref #389 (comment)

* commented out pending RTF equality comparisons with zones clarification
#389 (comment)

Co-authored-by: gregm <greg.mccreath@origin.com.au>
Co-authored-by: Matteo Mortari <matteo.mortari@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants