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.

Sign up for GitHub

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

[RFC] for single-attribute value types #409

Merged
merged 8 commits into from
Jan 22, 2024
Merged

[RFC] for single-attribute value types #409

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e5d6173
Trying my hands at an RFC
dirkriehle Jul 29, 2023
f56ed67
Formatted jayvee code correctly in MD file
dirkriehle Jul 29, 2023
aa59f57
Nicer markdown structure
dirkriehle Jul 29, 2023
38720e0
Added comment to avoid confusion over empty block
dirkriehle Sep 19, 2023
6c3f60f
Fixed inconsistencies with capital T in ValueType
dirkriehle Oct 31, 2023
cf1bcbc
Revert "Fixed inconsistencies with capital T in ValueType"
dirkriehle Oct 31, 2023
1b56627
Resolving merge conflict
dirkriehle Oct 31, 2023
b18e3ae
Merge branch 'main' into main
dirkriehle Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions rfc/0014-attribute-based-valuetypes/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<!--
SPDX-FileCopyrightText: 2023 Friedrich-Alexander-Universitat Erlangen-Nurnberg

SPDX-License-Identifier: AGPL-3.0-only
-->

# RFC 0014: Attributed-based syntax for defining value types

| | |
|---|---|
| Feature Tag | `attribute-based value types` | <!-- TODO: choose a unique and declarative feature name -->
| Status | `DRAFT` | <!-- Possible values: DRAFT, DISCUSSION, ACCEPTED, REJECTED -->
| Responsible | `dirkriehle` | <!-- TODO: assign yourself as main driver of this RFC -->
<!--
Status Overview:
- DRAFT: The RFC is not ready for a review and currently under change. Feel free to already ask for feedback on the structure and contents at this stage.
- DISCUSSION: The RFC is open for discussion. Usually, we open a PR to trigger discussions.
- ACCEPTED: The RFC was accepted. Create issues to prepare implementation of the RFC.
- REJECTED: The RFC was rejected. If another revision emerges, switch to status DRAFT.
-->

## Summary

I'd like to allow the definition of user-defined single-attribute value types. In addition to the current syntax, in which an underlying value type is referenced through 'oftype', it does not use instantiation/inheritance but rather composition.

## Motivation

The purpose of using composition is that it is

1. more similar to traditional approaches and
2. is needed anyway for multi-attribute value types.

## Explanation

To define a value type like CorrelationCoefficient and its range of -1 to +1, we have to write:

```jayvee
valuetype CorrelationCoefficient oftype decimal {
constraints: [
MinusOneToPlusOneRange,
];
}
```

This syntax is smart in that you don't have to list and name an attribute but rather rely on an implicit 'value' attribute. Still, I propose to make that attribute explicit. New syntax would be:
Copy link
Member

Choose a reason for hiding this comment

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

Does this implicitly mean that single-attribute valuetypes have to define the attribute value?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should enforce this, the less naming magic the better. We can assume for a transition period that if a value type has only one attribute, that attribute is mapped to the value keyword in expressions for constraints.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the current syntax, I think there is no name, because it is implicit? In my proposed syntax, you can give it any name you want. It does not have to be value.


```jayvee
valuetype CorrelationCoefficient {
attributes: [
Copy link
Member

@georg-schwarz georg-schwarz Jul 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Question on how to name this concept: attributes vs. properties?

Copy link
Member Author

Choose a reason for hiding this comment

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

No strong feelings. I used attributes because that's what I know it by. Let me ask ChatGPT for any subtle semantic differences between attribute and property. Try this: "Hi! Is there any significant semantic difference between the words attribute and property when describing something?"

Copy link
Contributor

Choose a reason for hiding this comment

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

(I'd argue for following the existing naming of properties here as well with no good case for the other direction).

value oftype decimal;
];
constraints: [
MinusOneToPlusOneRange; // Don't know how to attach this to value attribute
Copy link
Member

@georg-schwarz georg-schwarz Jul 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If we refactor the constraints, the constraints could always refer to the complete valuetype (atomic or composite). This would also allow constraints spanning different attributes of a valuetype.

Probably a bad example with the alternative flat syntax I proposed in another comment:

valuetype SumIsOne {
    attribute x1 oftype decimal;
    attribute x2 oftype decimal;

    constraint sumIsOne: x1 + x2 == 1.0;
    constraint x1isPositive: x1 > 0;
    constraint x2isPositive: x2 > 0;
}

Copy link
Member

Choose a reason for hiding this comment

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

Alternative, with the current constraints, we could introduce sth like this on keyword:

valuetype CorrelationCoefficient {
    attributes: [
        value oftype decimal,
    ];
    constraints: [
        MinusOneToPlusOneRange on value,
    ]
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the first flat syntax is nice, it could work with references constraints by instantiating them and assigning the values from attributes from them.

Consider:

constraint IsPositive on integer: value >= 0;

valuetype SumIsOne {
    attribute x1 oftype decimal;
    attribute x2 oftype decimal;

    constraint sumIsOne: x1 + x2 == 1.0;
    constraint x1isPositive: IsPositive(value = x1);
    constraint x2isPositive: IsPositive(value = x2);
}

This is a pretty big change because it introduces syntax for instantiating a constraint and assigning values to it but I'd like to throw it out there for discussion. Some syntax alternatives for that are possible of course, but I also like explicit parameter naming.

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 on "flat syntax" over collections (keyword in plural form) if we can have only one.

On Philip's example I observe a subtle syntactic incongruence I think which is the use of : for constraint but not attribute.

Copy link
Contributor

Choose a reason for hiding this comment

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

We had this discussion before I think and it is true. My intuition is that we describe a property of the SumIsOne valuetype with both (attribute x1 oftype decimal is a property that is of type decimal attribute with the name x1, constraint sumIsOne is a property of type constraint with the name sumIsOne). The syntactic difference then comes from assigning a value to one (the constraint has an expression assigned) and assigning no value to the attribute, just declaring it's existence.

];
}
```

While more verbose, it prepares the way for

```jayvee
valuetype Money {
attributes: [
amount oftype decimal;
currency oftype Currency;
];
Comment on lines +62 to +65
Copy link
Member

Choose a reason for hiding this comment

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

Seems very intuitive!

I'd like to show an alternative syntax for this that I personally like more:

valuetype Money {
  attribute amount oftype decimal;
  attribute currency oftype Currency;
}

This is probably sth very general to discuss on language design: keep the syntax flat or nested? I think here, the flat approach is more natural and similar to other languages.

Copy link
Contributor

Choose a reason for hiding this comment

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

We've used property instead of attributes in built-in blocks (https://github.com/jvalue/jayvee/tree/main/rfc/0008-builtin-blocktypes). I'd use the same naming here. For blocks, we decided properties are values an object owns intrinsically that you use in modeling, attributes is something you attribute to objects from the outside. The distinction is very faint but I would argue for consistency and reuse property here.

Otherwise I think the syntax is nice and would go for:

valuetype Money {
  property amount oftype decimal;
  property currency oftype Currency;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

At first glance, I'm indifferent/don't see a difference between property and attribute. Is there a semantic difference?

Which ever keyword we use, I observe a pattern of singular plural i.e. property/properties and constraint/constraints.

My take is that users may want both (property for one new property, and properties for a list of properties, constraint for one constraint, and constraints for a set of constraints). If only one is possible, I'd go for singular i.e. Philip's suggestion

}
```
which we'll need anyway. Conceivably, the step to multi-attribute value types could be merged with this one, but I simply wanted to try the RFC process rather than keep sending email ;-)

<!--
TODO: Explain the details of the RFC.
If the RFC contains more than a single cohesive aspect, structure this section accordingly.
Make sure to provide realistic modelling examples on the example data set introduced above.
-->

## Drawbacks

1. Introduces a redundant syntax,
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, we could replace the current syntax with it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also replace the syntax.

Copy link
Member Author

Choose a reason for hiding this comment

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

The beauty of not much legacy yet ;-) Agreed.

2. Creates extra work/may be too difficult, and
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it would be too difficult ;-)

One issue ahead is the attribute name value as we use value as keyword in expression-based constraints. These constraints have to be redesigned then to rather define parameters in a lambda-like way, which is a good idea anyway.

3. May be disruptive to how the language currently works.
Copy link
Member

Choose a reason for hiding this comment

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

IMO it blends in well ;-)


## Alternatives

Conceivably, we could use multiple inheritance for multi-attribute value types... just joking.
Copy link
Member

Choose a reason for hiding this comment

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

But how about inheritance in general?

Right now, this syntax does not allow an extension of an existing valuetype. I would see value offering it, though:

valuetype PositiveCorrelationCoefficient extends CorrelationCoefficient {
    // inherits all attributes
    // could add further attributes?

    // inherits all constraints
    // could ad further constraints
    constraints: [
         ZeroToPlusOneRange,
    ]
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vote for keeping inheritance out of this RFC to have a smaller scope :).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm all for single inheritance but agree that it is not part of this RFC.


## Possible Future Changes/Enhancements

This proposal is to prepare the way for multi-attribute value types.
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to point out the interplay with generics that we currently need to implement (without behavior yet) to represent collections in the blocktype definition.

Here a generic example since I don't have a good usecase for it in mind yet.

valuetype MyGenericValuetype<T> {
  parameters: [
     x oftype text,
     y oftype T
  ]
}

Copy link
Member Author

Choose a reason for hiding this comment

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

We really should put this to the test in AMSE/SAKI/MADE. I have always suspected that beyond very simple uses of generics, folks don't get it/don't use it because it makes their brain jump out of its casing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with this, for some use cases generics are basically required though (e.g., for our self description using built-in blocks). But we do not have to expose that much of it to users. We should aim for empirically verifying that our hypotheses are correct though, yeah.

Loading