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

Add default container properties #402

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

davaya
Copy link
Contributor

@davaya davaya commented Jun 29, 2023

Per @zvr in issue #393, the default values of minCount and maxCount are implicitly:

  • minCount: 0
  • maxCount: *

This PR adds these defaults explicitly to the model, and adds default values applicable to properties where maxCount is greater than 1:

  • isOrdered: false
  • isUnique: true
  • isOptional: false

The four combinations of isOrdered and isUnique specify the container type:

Type isOrdered isUnique
Set false true
List true false
Record true true
Bag false false

The default type is Set, where duplicate values are an error and ordering does not affect equality: [a, b] is equal to [b, a]. Ordered lists and ordered sets are also common but less frequently used - where isOrdered is true, [a, b] is not equal to [b, a]. Bag is rare, but if [a, a, b] must be permitted as a valid value and [a, a, b] is equal to [a, b, a] and not equal to [a, b], the model can define it. These defaults will rarely be overridden by type definitions, but if it is necessary to define a list of values where order matters, it should be possible to do so.

If set to true, isOptional declares that a property with maxCount > 1 may be omitted regardless of the value of minCount. For example a property x may have minCount=3, maxCount=5, and isOptional=true to declare that the property may be omitted but if present must have the specified item count. This is normally used with minCount=1, maxCount=*, isOptional=true to eliminate the ambiguity of having two different values of nil:

  • property x omitted, and
  • property x present with value [ ].

Copy link
Member

@maxhbr maxhbr left a comment

Choose a reason for hiding this comment

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

can you please specify how your proposed properties would translate to RDF. Maybe best implement them in the spec-parser first, before adding them to the files that are used by the spec-parser.

And as far as I understand the underlying semantics, that we inherit from our strong reliance and coupling to RDF, I think the default type might be what you call Bag.

Also not sure if that is the right place to put these things. Profiles probably not want to overwrite that (and giving them the possibility feels weird). It also does not feel like a profile property. I think it is more an inherent property of the notation we use. So it Probably would fit best in the non-existing documentation that specifies the semantics of the .md files.

@davaya
Copy link
Contributor Author

davaya commented Jul 9, 2023

The logical model is independent of all serializations, not specific to RDF or to JSON. The logical class notation is independent of both serializations and programming languages - Armin is implementing them in Python code, but the notion of "class", "property" and "type" aren't Python-specific.

UML section 7.5 defines Multiplicity Elements (not SPDX Elements, but generic containers) with the four container properties.

RDF also supports containers, about which the 1999 spec says:

The definitions of Bag and Sequence explicitly permit duplicate values. RDF does not define a core concept of Set, which would be a Bag with no duplicates, because the RDF core does not mandate an enforcement mechanism in the event of violations of such constraints. Future work layered on the RDF core may define such facilities.

Sean should be able to answer if RDF has matured in the last 24 years to support sets. It's hard to believe it does not, but even if it doesn't, that's a matter of enforcement, not syntax. The model files can define all four container types and particular serializations can ignore enforcement if they don't support it.

There's a huge benefit to supporting isUnique: a List or Bag with [1..*] could contain hundreds or millions of items, but if CreationInfo/profile is a Set or Record with [1..*] items, it can contain no more than 8. There is no need to count the number of profiles defined in the model, set maxCount to 8, and update it each time a new profile is defined, you can leave maxCount=* and isUnique=true and the valid serialized data automatically tracks the number of profiles.

Also not sure if that is the right place to put these things. Profiles probably not want to overwrite that (and giving them the possibility feels weird).

I agree, and am open to suggestions for where best to include them in the model.

Right now Core says "The Core namespace defines foundational concepts serving as the basis for all SPDX-3.0 profiles.", which means the alternatives are:

  1. use the existing syntax along with a statement that profiles shall not override them, or
  2. make up a new and different syntax that would have to be defined and supported.

It seems easier to enforce the no-override policy in software than to make up something different, but either way is OK.

@goneall
Copy link
Member

goneall commented Jul 9, 2023

The logical model is independent of all serializations, not specific to RDF or to JSON.

I think one of our disconnects is thinking of RDF as a serialization. RDF is a data model with several different serialization formats. Reference Wikipedia definition of RDF.

@goneall
Copy link
Member

goneall commented Jul 9, 2023

After leaving that last comment, I though I should follow up with my opinion that RDF is a data model we should support but I don't believe it is perfect and I'm quite open to supporting additional data models if they solve use cases RDF does not solve.

@kestewart kestewart requested a review from goneall July 28, 2023 19:23
@kestewart kestewart modified the milestones: 3.0-rc2, 3.0 Jul 28, 2023
@goneall goneall added the documentation Improvements or additions to documentation label Jul 28, 2023
@kestewart
Copy link
Contributor

@maxhbr - have the changes you were looking for been made?

@nishakm
Copy link
Collaborator

nishakm commented Jan 16, 2024

@davaya Would this be covered by ElementCollection in the current model as of 2024-01-16?

@maxhbr
Copy link
Member

maxhbr commented Jan 16, 2024

No, I think that support for isOrdered, isUnique and isOptional is not yet supported by spec-parser and this PR contains no documentation that describes their semantics. These come also from the object oriented view of the model, and RDF persons might disagree on the concept.

@goneall
Copy link
Member

goneall commented Apr 3, 2024

Since this isn't supported by the spec parser, I'm moving this to a 3.1 release for consideration

@goneall goneall modified the milestones: 3.0, 3.1 Apr 3, 2024
@bact bact added the enhancement New feature or request label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants