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

model: Core: DateTime: Inherit from xsd:dateTime #661

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

JPEWdev
Copy link
Contributor

@JPEWdev JPEWdev commented Mar 8, 2024

Inherit from xsd:dateTime instead of xsd:string. The XSD definition is compatible with the SPDX definition, and this will allow code generation tools to map the value to a code-native datatype.

@zvr
Copy link
Member

zvr commented Mar 8, 2024

👎 I disagree.

All our "Datatypes" are xsd:string with a pattern defining the valid format.

Note that Core/DateTime does not allow everything that xsd:dateTime does.

@JPEWdev
Copy link
Contributor Author

JPEWdev commented Mar 8, 2024

The SPDX DateTime pattern is a strict subset of the allowed xsd:dateTime string, so it's not incompatible, it would just be a further restriction.

The nice part about using xsd:dateTime is that it means language bindings can know to use a native datetime type which is convenient for users; otherwise it just looks like another string.

@zvr
Copy link
Member

zvr commented Mar 14, 2024

Very briefly:

  • I don't even know how dateTime values are represented in JSON-LD (asthey are not strings). I know that in other serializations, e.g. TTL, you have to always use something like "2024-03-14T12:34:56"^^xsd:dateTime and this is not very convenient.
  • I am also not sure whether we can have SHACL pattern restrictions on non-string values. The SHACL online playground does not seem to support it, but I don't know whether this is a restriction of the implementation or the SHACL spec. On the other hand, if there are no validators that support it, the point is moot...

@goneall
Copy link
Member

goneall commented Mar 17, 2024

I did a bit of research on xsd:dateTime:

  • It is a subtype of xsd:anySimpleType which is the base type for both dateTime and string - so it would be incompatible to switch this - probably a decision we need to make in 3.0
  • You are allowed to further restrict xsd:dateTime with a pattern
  • Based on several stackoverflow articles, I concluded that it is not common to restrict xsd:dateTime with a pattern, so we may get somewhat mixed results from some XML and RDF parsers

Based on the above I'm leaning towards changing the type inheritance to xsd:dateTime as proposed.

References:

@goneall goneall added this to the 3.0-rc3 milestone Mar 17, 2024
Inherit from xsd:dateTimeStamp instead of xsd:string. The XSD definition
is compatible with the SPDX definition, and this will allow code
generation tools to map the value to a code-native datatype.
@kestewart
Copy link
Contributor

Mostly agreed to in 3/19 meeting, if @zvr doesn't comment further by 3/28 - will be merged.

@kestewart
Copy link
Contributor

Alexios joined, and agreed that decision made last week can stand. Merging.

@kestewart kestewart merged commit efe92f4 into spdx:main Mar 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants