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

Codemeta v3.0 - generation #34

Merged
merged 9 commits into from
Apr 29, 2024
Merged

Conversation

hjonin
Copy link
Contributor

@hjonin hjonin commented Mar 27, 2024

Relates to #23.

Allow codemeta.json v3.0 file generation.

Still TODO:

@moranegg I'm attaching a screenshot so you can also validate the interface

Screenshot 2024-03-27 at 15-56-08 CodeMeta generator

hjonin added 2 commits March 26, 2024 11:14
* Expand jsonld from custom context
* Resolves codemeta#29
* missing tests for v3.0
* missing import for v3.0 (and check session storage is working)
* extra validation? (eg. end date > start date)
@hjonin
Copy link
Contributor Author

hjonin commented Apr 3, 2024

@moranegg @progval Should the generator now generate v3.0 by default (on value change)? If so, I'm also updating previous tests.
The other solution would be a switch button at the beginning, which I'm not found of: I think it's better to reduce the user cognitive load, and to encourage using the v3.0 by default.

@progval
Copy link
Member

progval commented Apr 3, 2024

Good question. I think we should wait for Codemeta parsers to support v3.0. There's swh-indexer and a few listed here: https://codemeta.github.io/tools/ ; none of them seem to support v3.0 yet.

@hjonin
Copy link
Contributor Author

hjonin commented Apr 3, 2024

Another question, should we add validation on cross terms, e.g. fix

if (fieldName.startsWith("codemeta:") || fieldName.startsWith("schema:")) {
    // Do not check fields from other versions FIXME ?
    return true;
}

?

@progval
Copy link
Member

progval commented Apr 3, 2024

The current version of the generator refuses unknown fields. Whether that's a good idea is debatable; but we shouldn't change this behavior in a PR that is about something else (adding support for Codemeta v3). Instead, validation should allow both v2 and v3 terms, and reject everything else.

@hjonin
Copy link
Contributor Author

hjonin commented Apr 4, 2024

I'm not sure to understand what do you mean by

validation should allow both v2 and v3 terms, and reject everything else.

What should it allow, and what should it reject, in terms of cross version terms (e.g. contIntegration/continuousIntegration/codemeta:contIntegration/codemeta:continuousIntegration)?

@progval
Copy link
Member

progval commented Apr 4, 2024

It should allow both https://codemeta.github.io/terms/contIntegration and https://codemeta.github.io/terms/continuousIntegration. And because this PR still compacts with the v2 context (doc = await jsonld.compact(parsed, CODEMETA_CONTEXTS["2.0"].url); // Only import codemeta v2.0 for now), they are compacted respectively as contIntegration and codemeta:continuousIntegration.

@hjonin
Copy link
Contributor Author

hjonin commented Apr 4, 2024

So, it should always fail in error, like this?
image
image

@progval
Copy link
Member

progval commented Apr 4, 2024

No, these two are semantically the same document (they both expand to the exact same document), and they are both valid, because it is valid to use both https://codemeta.github.io/terms/contIntegration and https://codemeta.github.io/terms/continuousIntegration in the same document regardless of context.

@hjonin
Copy link
Contributor Author

hjonin commented Apr 4, 2024

Ah ok, so for you it was ok to have this code without further validation?

@progval
Copy link
Member

progval commented Apr 4, 2024

It's not, because codemeta:abcde should be disallowed, for example.

@hjonin
Copy link
Contributor Author

hjonin commented Apr 4, 2024

Ah, I think I understand now. So, would it be ok/sufficient to have in validators:

"contIntegration": validateUrls,
"continuousIntegration": validateUrls,
"codemeta:contIntegration": noValidation,
"codemeta:continuousIntegration": noValidation,

@progval
Copy link
Member

progval commented Apr 4, 2024

When compacting with a v2 context, you would never get a continuousIntegration property (because continuousIntegration is not in the v2 context) and you would only get a codemeta:contIntegration property if it does not have type @id. So:

  • contIntegration -> validate URLs
  • continuousIntegration -> can't happen
  • codemeta:contIntegration -> always reject
  • codemeta:continuousIntegration -> validate URLs

@progval
Copy link
Member

progval commented Apr 4, 2024

It would be less confusing if we expanded the document before validating (ie. #29), instead of compacting with the v2 context. In particular, it would break this asymmetry, and the validation simply becomes:

"https://codemeta.github.io/terms/contIntegration": validateUrls,
"https://codemeta.github.io/terms/continuousIntegration": validateUrls,

@hjonin
Copy link
Contributor Author

hjonin commented Apr 4, 2024

Do you mean that https://github.com/hjonin/codemeta-generator/blob/3c76b4b876df2de62e6f88eaeedc1bdfa0437a03/js/validation/index.js#L16

function validateDocument(doc) {

should take the expanded document for input (I did not get that at first for #29)?

@progval
Copy link
Member

progval commented Apr 4, 2024

yes, exactly

@hjonin
Copy link
Contributor Author

hjonin commented Apr 4, 2024

ok :) I'll do that in another PR indeed

@hjonin hjonin changed the title Codemeta v3.0 Codemeta v3.0 - generation Apr 5, 2024
@hjonin hjonin mentioned this pull request Apr 5, 2024
@moranegg
Copy link
Collaborator

Excellent discussion and work!
Is the PR ready for merge?

@moranegg moranegg requested a review from progval April 12, 2024 09:51
Copy link
Member

@progval progval left a comment

Choose a reason for hiding this comment

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

Thanks, looks great

cypress/integration/persons.js Outdated Show resolved Hide resolved
cypress/integration/persons.js Outdated Show resolved Hide resolved
Comment on lines +445 to +454
{
"type": "schema:Role",
"schema:author": {
"type": "Person",
"givenName": "Jane"
},
"schema:roleName": "Developer",
"schema:startDate": "2024-03-04",
"schema:endDate": "2024-04-03"
}
Copy link
Member

Choose a reason for hiding this comment

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

This won't be readable by v2-only parsers.

I believe we should return both formats, ie.

                "author": [
                    {
                        "type": "schema:Role",
                        "schema:author": {
                            "type": "Person",
                            "givenName": "Jane"
                        },
                        "schema:roleName": "Developer",
                        "schema:startDate": "2024-03-04",
                        "schema:endDate": "2024-04-03"
                    },
                    {
                        "type": "Person",
                        "givenName": "Jane"
                    }

or possibly only the latter in the v2 export.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've allowed the first solution in the last commit.

Comment on lines 98 to 101
else if (fieldName.startsWith("codemeta:") || fieldName.startsWith("schema:")) {
// Do not check fields from other versions FIXME ?
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you remind me why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could happen when validating of multi-version document, where, for instance, in this case:

    {
              "@context": "https://w3id.org/codemeta/3.0",
              "type": "SoftwareSourceCode",
              "name": "My Test Software",
              "continuousIntegration": "https://test-ci.org/my-software",
              "codemeta:contIntegration": {
                  "id": "https://test-ci.org/my-software"
              }
      }

continuousIntegration could be validated but not codemeta:contIntegration. But this hack is removed with hjonin#1.

Copy link
Member

Choose a reason for hiding this comment

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

Right. But then the "Unknown field" is never triggered because this addition allows every unknown property in the codemeta and schema.org namespaces.

Could you make this hack more specific, by explicitly listing the few properties that were added/removed? (and then remove it in the future of course)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in the last commit.

hjonin and others added 3 commits April 29, 2024 15:04
@progval
Copy link
Member

progval commented Apr 29, 2024

Thanks!

@progval progval merged commit bb4b28c into codemeta:master Apr 29, 2024
1 check passed
@hjonin hjonin deleted the feat/gh-23-codemeta-v3 branch May 7, 2024 08:15
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

Successfully merging this pull request may close these issues.

3 participants